From: "Heiko Stübner" <heiko@sntech.de> To: Lin Huang <hl@rock-chips.com> Cc: mark.yao@rock-chips.com, myungjoo.ham@samsung.com, cw00.choi@samsung.com, airlied@linux.ie, mturquette@baylibre.com, dbasehore@chromium.org, sboyd@codeaurora.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, dianders@chromium.org, linux-rockchip@lists.infradead.org, kyungmin.park@samsung.com, linux-arm-kernel@lists.infradead.org, tixy@linaro.org, xsf@rock-chips.com, typ@rock-chips.com Subject: Re: [PATCH v3 1/7] firmware: rockchip: sip: Add rockchip SIP runtime service Date: Fri, 22 Jul 2016 12:00:24 +0200 [thread overview] Message-ID: <1742973.lJ4EpAnn7j@diego> (raw) In-Reply-To: <1469178440-4668-2-git-send-email-hl@rock-chips.com> Hi, Am Freitag, 22. Juli 2016, 17:07:14 schrieb Lin Huang: > From: Shengfei xu <xsf@rock-chips.com> > > This patch adds support for the SiP interface, we can pass dram > paramtert to bl31, and control ddr frequency scaling in bl31. > > Signed-off-by: Shengfei xu <xsf@rock-chips.com> > Signed-off-by: Lin Huang <hl@rock-chips.com> [...] > +++ b/drivers/firmware/rockchip_sip.c > @@ -0,0 +1,64 @@ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * 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. > + * > + * Copyright (C) 2016 ARM Limited > + */ > +#include <linux/errno.h> > +#include <linux/linkage.h> > +#include <linux/of.h> > +#include <linux/pm.h> > +#include <linux/printk.h> > +#include "rockchip_sip.h" > + > +typedef unsigned long (psci_fn)(unsigned long, unsigned long, > + unsigned long, unsigned long); > +asmlinkage psci_fn __invoke_psci_fn_smc; > + > +#define CONFIG_DRAM_INIT 0x00 > +#define CONFIG_DRAM_SET_RATE 0x01 > +#define CONFIG_DRAM_ROUND_RATE 0x02 > +#define CONFIG_DRAM_SET_AT_SR 0x03 > +#define CONFIG_DRAM_GET_BW 0x04 > +#define CONFIG_DRAM_GET_RATE 0x05 > +#define CONFIG_DRAM_CLR_IRQ 0x06 > +#define CONFIG_DRAM_SET_PARAM 0x07 > + > +uint64_t sip_smc_ddr_init(void) if I'm reading the ATF writers guide [0] correctly, the SiP calls are soc- specific things: "For example, two SoC providers can use the same Function ID within the SiP Service calls OEN range to mean different things - as these calls should be specific to the SoC. " So I guess the function calls, as well as the constants should have a prefix. ROCKCHIP_SIP_DDR_FREQ, rockchip_sip_smc_set_ddr_param(), etc. Looking at Zynq in the ATF [1] seems to strengthen that theory with the similar constants having the ZYNQMP_ prefix. I guess it doesn't matter to much on the ATF side, as you build a soc-specific image, but on the kernel-side I think it should unambiguos, especially as functions and constants are part of a public header. I think the code itself looks fine though :-) Heiko [0] https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/rt-svc-writers-guide.md [1] https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/xilinx/zynqmp/sip_svc_setup.c > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, 0, > + 0, CONFIG_DRAM_INIT); > +} > + > +uint64_t sip_smc_set_ddr_param(uint64_t param) > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, param, > + 0, CONFIG_DRAM_SET_PARAM); > +} > + > +uint64_t sip_smc_set_ddr_rate(uint64_t rate) > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, rate, 0, > + CONFIG_DRAM_SET_RATE); > +} > + > +uint64_t sip_smc_get_ddr_rate(void) > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_GET_RATE); > +} > + > +uint64_t sip_smc_clr_ddr_irq(void) > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_CLR_IRQ); > +} > + > +uint64_t sip_smc_get_call_count(void) > +{ > + return __invoke_psci_fn_smc(SIP_SVC_CALL_COUNT, 0, 0, 0); > +} > diff --git a/drivers/firmware/rockchip_sip.h > b/drivers/firmware/rockchip_sip.h new file mode 100644 > index 0000000..6487734 > --- /dev/null > +++ b/drivers/firmware/rockchip_sip.h > @@ -0,0 +1,59 @@ > +/* Copyright (c) 2010-2015, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * 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. > + */ > +#ifndef __SIP_INT_H > +#define __SIP_INT_H > + > +/* SMC function IDs for SiP Service queries */ > +#define SIP_SVC_CALL_COUNT 0x8200ff00 > +#define SIP_SVC_UID 0x8200ff01 > +#define SIP_SVC_VERSION 0x8200ff03 > +#define SIP_DDR_FREQ 0xC2000008 > + > +#if IS_ENABLED(CONFIG_ROCKCHIP_SIP) > +uint64_t sip_smc_set_ddr_rate(uint64_t rate); > +uint64_t sip_smc_get_ddr_rate(void); > +uint64_t sip_smc_clr_ddr_irq(void); > +uint64_t sip_smc_get_call_count(void); > +uint64_t sip_smc_ddr_init(void); > +uint64_t sip_smc_set_ddr_param(uint64_t param); > +#else > +static inline uint64_t sip_smc_set_ddr_rate(uint64_t rate) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_get_ddr_rate(void) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_clr_ddr_irq(void) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_get_call_count(void) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_ddr_init(void) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_set_ddr_param(uint64_t param) > +{ > + return 0; > +} > +#endif > +#endif
WARNING: multiple messages have this Message-ID (diff)
From: heiko@sntech.de (Heiko Stübner) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v3 1/7] firmware: rockchip: sip: Add rockchip SIP runtime service Date: Fri, 22 Jul 2016 12:00:24 +0200 [thread overview] Message-ID: <1742973.lJ4EpAnn7j@diego> (raw) In-Reply-To: <1469178440-4668-2-git-send-email-hl@rock-chips.com> Hi, Am Freitag, 22. Juli 2016, 17:07:14 schrieb Lin Huang: > From: Shengfei xu <xsf@rock-chips.com> > > This patch adds support for the SiP interface, we can pass dram > paramtert to bl31, and control ddr frequency scaling in bl31. > > Signed-off-by: Shengfei xu <xsf@rock-chips.com> > Signed-off-by: Lin Huang <hl@rock-chips.com> [...] > +++ b/drivers/firmware/rockchip_sip.c > @@ -0,0 +1,64 @@ > +/* > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * 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. > + * > + * Copyright (C) 2016 ARM Limited > + */ > +#include <linux/errno.h> > +#include <linux/linkage.h> > +#include <linux/of.h> > +#include <linux/pm.h> > +#include <linux/printk.h> > +#include "rockchip_sip.h" > + > +typedef unsigned long (psci_fn)(unsigned long, unsigned long, > + unsigned long, unsigned long); > +asmlinkage psci_fn __invoke_psci_fn_smc; > + > +#define CONFIG_DRAM_INIT 0x00 > +#define CONFIG_DRAM_SET_RATE 0x01 > +#define CONFIG_DRAM_ROUND_RATE 0x02 > +#define CONFIG_DRAM_SET_AT_SR 0x03 > +#define CONFIG_DRAM_GET_BW 0x04 > +#define CONFIG_DRAM_GET_RATE 0x05 > +#define CONFIG_DRAM_CLR_IRQ 0x06 > +#define CONFIG_DRAM_SET_PARAM 0x07 > + > +uint64_t sip_smc_ddr_init(void) if I'm reading the ATF writers guide [0] correctly, the SiP calls are soc- specific things: "For example, two SoC providers can use the same Function ID within the SiP Service calls OEN range to mean different things - as these calls should be specific to the SoC. " So I guess the function calls, as well as the constants should have a prefix. ROCKCHIP_SIP_DDR_FREQ, rockchip_sip_smc_set_ddr_param(), etc. Looking at Zynq in the ATF [1] seems to strengthen that theory with the similar constants having the ZYNQMP_ prefix. I guess it doesn't matter to much on the ATF side, as you build a soc-specific image, but on the kernel-side I think it should unambiguos, especially as functions and constants are part of a public header. I think the code itself looks fine though :-) Heiko [0] https://github.com/ARM-software/arm-trusted-firmware/blob/master/docs/rt-svc-writers-guide.md [1] https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/xilinx/zynqmp/sip_svc_setup.c > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, 0, > + 0, CONFIG_DRAM_INIT); > +} > + > +uint64_t sip_smc_set_ddr_param(uint64_t param) > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, param, > + 0, CONFIG_DRAM_SET_PARAM); > +} > + > +uint64_t sip_smc_set_ddr_rate(uint64_t rate) > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, rate, 0, > + CONFIG_DRAM_SET_RATE); > +} > + > +uint64_t sip_smc_get_ddr_rate(void) > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_GET_RATE); > +} > + > +uint64_t sip_smc_clr_ddr_irq(void) > +{ > + return __invoke_psci_fn_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_CLR_IRQ); > +} > + > +uint64_t sip_smc_get_call_count(void) > +{ > + return __invoke_psci_fn_smc(SIP_SVC_CALL_COUNT, 0, 0, 0); > +} > diff --git a/drivers/firmware/rockchip_sip.h > b/drivers/firmware/rockchip_sip.h new file mode 100644 > index 0000000..6487734 > --- /dev/null > +++ b/drivers/firmware/rockchip_sip.h > @@ -0,0 +1,59 @@ > +/* Copyright (c) 2010-2015, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * 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. > + */ > +#ifndef __SIP_INT_H > +#define __SIP_INT_H > + > +/* SMC function IDs for SiP Service queries */ > +#define SIP_SVC_CALL_COUNT 0x8200ff00 > +#define SIP_SVC_UID 0x8200ff01 > +#define SIP_SVC_VERSION 0x8200ff03 > +#define SIP_DDR_FREQ 0xC2000008 > + > +#if IS_ENABLED(CONFIG_ROCKCHIP_SIP) > +uint64_t sip_smc_set_ddr_rate(uint64_t rate); > +uint64_t sip_smc_get_ddr_rate(void); > +uint64_t sip_smc_clr_ddr_irq(void); > +uint64_t sip_smc_get_call_count(void); > +uint64_t sip_smc_ddr_init(void); > +uint64_t sip_smc_set_ddr_param(uint64_t param); > +#else > +static inline uint64_t sip_smc_set_ddr_rate(uint64_t rate) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_get_ddr_rate(void) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_clr_ddr_irq(void) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_get_call_count(void) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_ddr_init(void) > +{ > + return 0; > +} > + > +static inline uint64_t sip_smc_set_ddr_param(uint64_t param) > +{ > + return 0; > +} > +#endif > +#endif
next prev parent reply other threads:[~2016-07-22 10:00 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-07-22 9:07 [PATCH v3 0/7] rk3399 support ddr frequency scaling Lin Huang 2016-07-22 9:07 ` Lin Huang 2016-07-22 9:07 ` [PATCH v3 1/7] firmware: rockchip: sip: Add rockchip SIP runtime service Lin Huang 2016-07-22 9:07 ` Lin Huang 2016-07-22 9:07 ` Lin Huang 2016-07-22 10:00 ` Heiko Stübner [this message] 2016-07-22 10:00 ` Heiko Stübner 2016-07-22 20:50 ` Heiko Stübner 2016-07-22 20:50 ` Heiko Stübner 2016-07-24 8:00 ` hl 2016-07-24 8:00 ` hl 2016-07-25 17:36 ` Sudeep Holla 2016-07-25 17:36 ` Sudeep Holla 2016-07-26 1:13 ` hl 2016-07-26 1:13 ` hl 2016-07-22 21:32 ` kbuild test robot 2016-07-22 21:32 ` kbuild test robot 2016-07-22 21:32 ` kbuild test robot 2016-07-26 18:29 ` Mark Rutland 2016-07-26 18:29 ` Mark Rutland 2016-07-22 9:07 ` [PATCH v3 2/7] clk: rockchip: add new clock-type for the ddrclk Lin Huang 2016-07-22 9:07 ` Lin Huang 2016-07-24 9:09 ` Heiko Stübner 2016-07-24 9:09 ` Heiko Stübner 2016-07-24 9:09 ` Heiko Stübner 2016-07-22 9:07 ` [PATCH v3 3/7] clk: rockchip: rk3399: add SCLK_DDRCLK ID for ddrc Lin Huang 2016-07-22 9:07 ` Lin Huang 2016-07-22 10:08 ` Heiko Stübner 2016-07-22 10:08 ` Heiko Stübner 2016-07-22 9:07 ` [PATCH v3 4/7] clk: rockchip: rk3399: add ddrc clock support Lin Huang 2016-07-22 9:07 ` Lin Huang 2016-07-22 9:07 ` [PATCH v3 5/7] PM / devfreq: event: support rockchip dfi controller Lin Huang 2016-07-22 9:07 ` Lin Huang 2016-07-22 20:28 ` Paul Gortmaker 2016-07-22 20:28 ` Paul Gortmaker 2016-07-22 20:28 ` Paul Gortmaker 2016-07-22 9:07 ` [PATCH v3 6/7] PM / devfreq: rockchip: add devfreq driver for rk3399 dmc Lin Huang 2016-07-22 9:07 ` Lin Huang 2016-07-22 20:24 ` Paul Gortmaker 2016-07-22 20:24 ` Paul Gortmaker 2016-07-22 20:24 ` Paul Gortmaker 2016-07-24 7:54 ` hl 2016-07-24 7:54 ` hl 2016-07-24 7:54 ` hl 2016-07-25 6:01 ` Chanwoo Choi 2016-07-25 6:01 ` Chanwoo Choi 2016-07-25 8:47 ` hl 2016-07-25 8:47 ` hl 2016-07-25 9:45 ` Chanwoo Choi 2016-07-25 9:45 ` Chanwoo Choi 2016-07-26 1:15 ` hl 2016-07-26 1:15 ` hl 2016-07-22 9:07 ` [PATCH v3 7/7] drm/rockchip: Add dmc notifier in vop driver Lin Huang 2016-07-22 9:07 ` Lin Huang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1742973.lJ4EpAnn7j@diego \ --to=heiko@sntech.de \ --cc=airlied@linux.ie \ --cc=cw00.choi@samsung.com \ --cc=dbasehore@chromium.org \ --cc=dianders@chromium.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=hl@rock-chips.com \ --cc=kyungmin.park@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-rockchip@lists.infradead.org \ --cc=mark.yao@rock-chips.com \ --cc=mturquette@baylibre.com \ --cc=myungjoo.ham@samsung.com \ --cc=sboyd@codeaurora.org \ --cc=tixy@linaro.org \ --cc=typ@rock-chips.com \ --cc=xsf@rock-chips.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.