Devicetree
 help / color / mirror / Atom feed
* RE: [PATCH 3/5] Documentation: dt: mtd: add chip support for "jedec, spi-nor"
From: Yao Yuan @ 2016-12-13  2:46 UTC (permalink / raw)
  To: Rob Herring
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	shawnguo@kernel.org, linux-kernel@vger.kernel.org,
	computersforpeace@gmail.com, dwmw2@infradead.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20161212170903.ixr6iu4c5txzreub@rob-hp-laptop>

On Thu, Dec 13, 2016 at 05:23:02PM +0800, Rob Herring wrote:
> On Thu, Dec 08, 2016 at 05:23:02PM +0800, Yuan Yao wrote:
> > From: Yuan Yao <yao.yuan@nxp.com>
> 
> The compatible string is wrong in the subject.
> 
> >
> > "sst25wf040b" and "en25s64" are also chip compatible with SPI NOR flash.
> >
> > Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> Otherwise,
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks for your review.
I will update the subject and send v2 soon.

^ permalink raw reply

* RE: [PATCH 5/5] Documentation: fsl-quadspi: Add fsl, ls1012a-qspi compatible string
From: Yao Yuan @ 2016-12-13  2:47 UTC (permalink / raw)
  To: Rob Herring, Yuan Yao
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	shawnguo@kernel.org, linux-kernel@vger.kernel.org,
	computersforpeace@gmail.com, dwmw2@infradead.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20161212170948.fq7v5lva4kgt6hly@rob-hp-laptop>

On Thu, Dec 13, 2016 at 05:23:02PM +0800, Rob Herring wrote:
> On Thu, Dec 08, 2016 at 05:23:04PM +0800, Yuan Yao wrote:
> > From: Yuan Yao <yao.yuan@nxp.com>
> 
> Same problem in this subject too.
> 
> >
> > new compatible string: "fsl,ls1012a-qspi".
> >
> > Signed-off-by: Yuan Yao <yao.yuan@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/mtd/fsl-quadspi.txt | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Acked-by: Rob Herring <robh@kernel.org>

Thanks for your review.
And do you have any suggestion for this subject?

I will update the subject and send v2 soon.

^ permalink raw reply

* Re: [PATCH v6] pwm: add pwm driver for HiSilicon BVT SOCs
From: Jian Yuan @ 2016-12-13  3:29 UTC (permalink / raw)
  To: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q,
	kevin.lixu-C8/M+/jPZTeaMJb+Lgu22Q,
	jalen.hsu-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <1480326166-145999-1-git-send-email-yuanjian12-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>

Hi, Thierry,
  Are there any issues with the PWM driver below.
  I would like to request some comment and advice form you, if you had reviewed the driver code.

  With best wishes,
  Jian Yuan

On 2016/11/28 17:42, Jian Yuan wrote:
> From: yuanjian <yuanjian12-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> 
> Add PWM driver for the PWM controller found on HiSilicon BVT SOCs, like Hi3519V100, Hi3516CV300, etc.
> The PWM controller is primarily in charge of controlling P-Iris lens.
> 
> Reviewed-by: Jiancheng Xue <xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> Signed-off-by: Jian Yuan <yuanjian12-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> 
> Change Log:
> v6:
> It supports polarity specified in DTB and #pwm-cells can be set to 3.
> v5:
> remove the generic compatible string "hisilicon, hibvt-pwm".
> v4:
> Add #pwm-cells in the bindings document.
> v3:
> fixed issues pointed by thierry.
> Add PWM compatible string for Hi3519V100.
> Implement .apply() function which support atomic, instead of .enable()/.disable()/.config().
> v2:
> The number of PWMs is change to be probeable based on the compatible string.
> 
>  .../devicetree/bindings/pwm/pwm-hibvt.txt          |  22 ++
>  drivers/pwm/Kconfig                                |   9 +
>  drivers/pwm/Makefile                               |   1 +
>  drivers/pwm/pwm-hibvt.c                            | 271 +++++++++++++++++++++
>  4 files changed, 302 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-hibvt.txt
>  create mode 100644 drivers/pwm/pwm-hibvt.c
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-hibvt.txt b/Documentation/devicetree/bindings/pwm/pwm-hibvt.txt
> new file mode 100644
> index 0000000..fa7849d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-hibvt.txt
> @@ -0,0 +1,21 @@
> +Hisilicon PWM controller
> +
> +Required properties:
> +-compatible: should contain one SoC specific compatible string
> + The SoC specific strings supported including:
> +	"hisilicon,hi3516cv300-pwm"
> +	"hisilicon,hi3519v100-pwm"
> +- reg: physical base address and length of the controller's registers.
> +- clocks: phandle and clock specifier of the PWM reference clock.
> +- resets: phandle and reset specifier for the PWM controller reset.
> +- #pwm-cells: Should be 3. See pwm.txt in this directory for a description of
> +  the cells format.
> +
> +Example:
> +	pwm: pwm@12130000 {
> +		compatible = "hisilicon,hi3516cv300-pwm";
> +		reg = <0x12130000 0x10000>;
> +		clocks = <&crg_ctrl HI3516CV300_PWM_CLK>;
> +		resets = <&crg_ctrl 0x38 0>;
> +		#pwm-cells = <3>;
> +	};
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index c182efc..b2d7408 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -158,6 +158,15 @@ config PWM_FSL_FTM
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pwm-fsl-ftm.
>  
> +config PWM_HIBVT
> +	tristate "HiSilicon BVT PWM support"
> +	depends on ARCH_HISI || COMPILE_TEST
> +	help
> +	  Generic PWM framework driver for HiSilicon BVT SoCs.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called pwm-hibvt.
> +
>  config PWM_IMG
>  	tristate "Imagination Technologies PWM driver"
>  	depends on HAS_IOMEM
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index dd35bc1..37ec39e 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_PWM_CLPS711X)	+= pwm-clps711x.o
>  obj-$(CONFIG_PWM_CRC)		+= pwm-crc.o
>  obj-$(CONFIG_PWM_EP93XX)	+= pwm-ep93xx.o
>  obj-$(CONFIG_PWM_FSL_FTM)	+= pwm-fsl-ftm.o
> +obj-$(CONFIG_PWM_HIBVT)		+= pwm-hibvt.o
>  obj-$(CONFIG_PWM_IMG)		+= pwm-img.o
>  obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
>  obj-$(CONFIG_PWM_JZ4740)	+= pwm-jz4740.o
> diff --git a/drivers/pwm/pwm-hibvt.c b/drivers/pwm/pwm-hibvt.c
> new file mode 100644
> index 0000000..d0e8f85
> --- /dev/null
> +++ b/drivers/pwm/pwm-hibvt.c
> @@ -0,0 +1,271 @@
> +/*
> + * PWM Controller Driver for HiSilicon BVT SoCs
> + *
> + * 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 <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/reset.h>
> +
> +#define PWM_CFG0_ADDR(x)    (((x) * 0x20) + 0x0)
> +#define PWM_CFG1_ADDR(x)    (((x) * 0x20) + 0x4)
> +#define PWM_CFG2_ADDR(x)    (((x) * 0x20) + 0x8)
> +#define PWM_CTRL_ADDR(x)    (((x) * 0x20) + 0xC)
> +
> +#define PWM_ENABLE_SHIFT    0
> +#define PWM_ENABLE_MASK     BIT(0)
> +
> +#define PWM_POLARITY_SHIFT  1
> +#define PWM_POLARITY_MASK   BIT(1)
> +
> +#define PWM_KEEP_SHIFT      2
> +#define PWM_KEEP_MASK       BIT(2)
> +
> +#define PWM_PERIOD_MASK     GENMASK(31, 0)
> +#define PWM_DUTY_MASK       GENMASK(31, 0)
> +
> +struct hibvt_pwm_chip {
> +	struct pwm_chip	chip;
> +	struct clk *clk;
> +	void __iomem *base;
> +	struct reset_control *rstc;
> +};
> +
> +struct hibvt_pwm_soc {
> +	u32 num_pwms;
> +};
> +
> +static const struct hibvt_pwm_soc pwm_soc[2] = {
> +	{ .num_pwms = 4 },
> +	{ .num_pwms = 8 },
> +};
> +
> +static inline struct hibvt_pwm_chip *to_hibvt_pwm_chip(struct pwm_chip *chip)
> +{
> +	return container_of(chip, struct hibvt_pwm_chip, chip);
> +}
> +
> +static void hibvt_pwm_set_bits(void __iomem *base, u32 offset,
> +					u32 mask, u32 data)
> +{
> +	void __iomem *address = base + offset;
> +	u32 value;
> +
> +	value = readl(address);
> +	value &= ~mask;
> +	value |= (data & mask);
> +	writel(value, address);
> +}
> +
> +static void hibvt_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
> +
> +	hibvt_pwm_set_bits(hi_pwm_chip->base, PWM_CTRL_ADDR(pwm->hwpwm),
> +			PWM_ENABLE_MASK, 0x1);
> +}
> +
> +static void hibvt_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
> +
> +	hibvt_pwm_set_bits(hi_pwm_chip->base, PWM_CTRL_ADDR(pwm->hwpwm),
> +			PWM_ENABLE_MASK, 0x0);
> +}
> +
> +static void hibvt_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> +					int duty_cycle_ns, int period_ns)
> +{
> +	struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
> +	u32 freq, period, duty;
> +
> +	freq = div_u64(clk_get_rate(hi_pwm_chip->clk), 1000000);
> +
> +	period = div_u64(freq * period_ns, 1000);
> +	duty = div_u64(period * duty_cycle_ns, period_ns);
> +
> +	hibvt_pwm_set_bits(hi_pwm_chip->base, PWM_CFG0_ADDR(pwm->hwpwm),
> +			PWM_PERIOD_MASK, period);
> +
> +	hibvt_pwm_set_bits(hi_pwm_chip->base, PWM_CFG1_ADDR(pwm->hwpwm),
> +			PWM_DUTY_MASK, duty);
> +}
> +
> +static void hibvt_pwm_set_polarity(struct pwm_chip *chip,
> +					struct pwm_device *pwm,
> +					enum pwm_polarity polarity)
> +{
> +	struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
> +
> +	if (polarity == PWM_POLARITY_INVERSED)
> +		hibvt_pwm_set_bits(hi_pwm_chip->base, PWM_CTRL_ADDR(pwm->hwpwm),
> +				PWM_POLARITY_MASK, (0x1 << PWM_POLARITY_SHIFT));
> +	else
> +		hibvt_pwm_set_bits(hi_pwm_chip->base, PWM_CTRL_ADDR(pwm->hwpwm),
> +				PWM_POLARITY_MASK, (0x0 << PWM_POLARITY_SHIFT));
> +}
> +
> +static void hibvt_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> +				struct pwm_state *state)
> +{
> +	struct hibvt_pwm_chip *hi_pwm_chip = to_hibvt_pwm_chip(chip);
> +	void __iomem *base;
> +	u32 freq, value;
> +
> +	freq = div_u64(clk_get_rate(hi_pwm_chip->clk), 1000000);
> +	base = hi_pwm_chip->base;
> +
> +	value = readl(base + PWM_CFG0_ADDR(pwm->hwpwm));
> +	state->period = div_u64(value * 1000, freq);
> +
> +	value = readl(base + PWM_CFG1_ADDR(pwm->hwpwm));
> +	state->duty_cycle = div_u64(value * 1000, freq);
> +
> +	value = readl(base + PWM_CTRL_ADDR(pwm->hwpwm));
> +	state->enabled = (PWM_ENABLE_MASK & value);
> +}
> +
> +static int hibvt_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> +				struct pwm_state *state)
> +{
> +	if (state->polarity != pwm->state.polarity)
> +		hibvt_pwm_set_polarity(chip, pwm, state->polarity);
> +
> +	if (state->period != pwm->state.period ||
> +		state->duty_cycle != pwm->state.duty_cycle)
> +		hibvt_pwm_config(chip, pwm, state->duty_cycle, state->period);
> +
> +	if (state->enabled != pwm->state.enabled) {
> +		if (state->enabled)
> +			hibvt_pwm_enable(chip, pwm);
> +		else
> +			hibvt_pwm_disable(chip, pwm);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct pwm_ops hibvt_pwm_ops = {
> +	.get_state = hibvt_pwm_get_state,
> +	.apply = hibvt_pwm_apply,
> +
> +	.owner = THIS_MODULE,
> +};
> +
> +static int hibvt_pwm_probe(struct platform_device *pdev)
> +{
> +	const struct hibvt_pwm_soc *soc =
> +				of_device_get_match_data(&pdev->dev);
> +	struct hibvt_pwm_chip *pwm_chip;
> +	struct resource *res;
> +	int ret;
> +	int i;
> +
> +	pwm_chip = devm_kzalloc(&pdev->dev, sizeof(*pwm_chip), GFP_KERNEL);
> +	if (pwm_chip == NULL)
> +		return -ENOMEM;
> +
> +	pwm_chip->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(pwm_chip->clk)) {
> +		dev_err(&pdev->dev, "getting clock failed with %ld\n",
> +				PTR_ERR(pwm_chip->clk));
> +		return PTR_ERR(pwm_chip->clk);
> +	}
> +
> +	pwm_chip->chip.ops = &hibvt_pwm_ops;
> +	pwm_chip->chip.dev = &pdev->dev;
> +	pwm_chip->chip.base = -1;
> +	pwm_chip->chip.npwm = soc->num_pwms;
> +	pwm_chip->chip.of_xlate = of_pwm_xlate_with_flags;
> +	pwm_chip->chip.of_pwm_n_cells = 3;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	pwm_chip->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(pwm_chip->base))
> +		return PTR_ERR(pwm_chip->base);
> +
> +	ret = clk_prepare_enable(pwm_chip->clk);
> +	if (ret < 0)
> +		return ret;
> +
> +	pwm_chip->rstc = devm_reset_control_get(&pdev->dev, NULL);
> +	if (IS_ERR(pwm_chip->rstc)) {
> +		clk_disable_unprepare(pwm_chip->clk);
> +		return PTR_ERR(pwm_chip->rstc);
> +	}
> +
> +	reset_control_assert(pwm_chip->rstc);
> +	msleep(30);
> +	reset_control_deassert(pwm_chip->rstc);
> +
> +	ret = pwmchip_add(&pwm_chip->chip);
> +	if (ret < 0) {
> +		clk_disable_unprepare(pwm_chip->clk);
> +		return ret;
> +	}
> +
> +	for (i = 0; i < pwm_chip->chip.npwm; i++) {
> +		hibvt_pwm_set_bits(pwm_chip->base, PWM_CTRL_ADDR(i),
> +				PWM_KEEP_MASK, (0x1 << PWM_KEEP_SHIFT));
> +	}
> +
> +	platform_set_drvdata(pdev, pwm_chip);
> +
> +	return 0;
> +}
> +
> +static int hibvt_pwm_remove(struct platform_device *pdev)
> +{
> +	struct hibvt_pwm_chip *pwm_chip;
> +
> +	pwm_chip = platform_get_drvdata(pdev);
> +
> +	reset_control_assert(pwm_chip->rstc);
> +	msleep(30);
> +	reset_control_deassert(pwm_chip->rstc);
> +
> +	clk_disable_unprepare(pwm_chip->clk);
> +
> +	return pwmchip_remove(&pwm_chip->chip);
> +}
> +
> +static const struct of_device_id hibvt_pwm_of_match[] = {
> +	{ .compatible = "hisilicon,hi3516cv300-pwm", .data = &pwm_soc[0] },
> +	{ .compatible = "hisilicon,hi3519v100-pwm", .data = &pwm_soc[1] },
> +	{  }
> +};
> +MODULE_DEVICE_TABLE(of, hibvt_pwm_of_match);
> +
> +static struct platform_driver hibvt_pwm_driver = {
> +	.driver = {
> +		.name = "hibvt-pwm",
> +		.of_match_table = hibvt_pwm_of_match,
> +	},
> +	.probe = hibvt_pwm_probe,
> +	.remove	= hibvt_pwm_remove,
> +};
> +module_platform_driver(hibvt_pwm_driver);
> +
> +MODULE_AUTHOR("Jian Yuan");
> +MODULE_DESCRIPTION("HiSilicon BVT SoCs PWM driver");
> +MODULE_LICENSE("GPL");
> 

--
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 v2 2/2] dt-bindings: Add DT bindings info for FlexRM ring manager
From: Anup Patel @ 2016-12-13  3:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: Jassi Brar, Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR,
	Rob Rice, Device Tree, Linux Kernel, Linux ARM Kernel,
	BCM Kernel Feedback
In-Reply-To: <20161209180132.lnpvwybnij5uykuq@rob-hp-laptop>

On Fri, Dec 9, 2016 at 11:31 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Dec 02, 2016 at 10:08:56AM +0530, Anup Patel wrote:
>> This patch adds device tree bindings document for the FlexRM
>> ring manager found on Broadcom iProc SoCs.
>>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>> ---
>>  .../bindings/mailbox/brcm,iproc-flexrm-mbox.txt    | 60 ++++++++++++++++++++++
>>  1 file changed, 60 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
>> new file mode 100644
>> index 0000000..e81f116
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
>> @@ -0,0 +1,60 @@
>
> [...]
>
>> +Example:
>> +--------
>> +crypto_mbox: mbox@67000000 {
>> +     compatible = "brcm,flexrm-mbox";
>> +     reg = <0x67000000 0x200000>;
>> +     msi-parent = <&gic_its 0x7f00>;
>> +     #mbox-cells = <3>;
>> +};
>> +
>> +crypto_client {
>> +     ...
>> +     mboxes = <&crypto_mbox 0 0x1 0xffff>,
>> +              <&crypto_mbox 1 0x1 0xffff>,
>> +              <&crypto_mbox 16 0x1 0xffff>,
>> +              <&crypto_mbox 17 0x1 0xffff>,
>> +              <&crypto_mbox 30 0x1 0xffff>,
>> +              <&crypto_mbox 31 0x1 0xffff>;
>
> The FlexRM part looks fine. I still don't understand what this node is.
> Is this a h/w block or just a list of mailboxes? What determines the
> mailbox channel numbers here? I need to see what the complete node looks
> like.

The crypto_client is an example mailbox client for FlexRM mailbox.

The example shows that we have FlexRM ring manager providing
ring based programming interface to Crypto offload engine. The
Crypto offload engine driver is a mailbox client driver.

FlexRM mailbox requires 3 cells to specify each mailbox channel in
"mboxes" attribute of mailbox client DT node. The 1st cell is
mailbox channel number (or FlexRM ring number). Rest of the
cells are explained in bindings document.

Complete node would look something like below:

crypto_client {
     compatible = "brcm,iproc-spu2";
     mboxes = <&crypto_mbox 0 0x1 0xffff>,
                     <&crypto_mbox 1 0x1 0xffff>,
                     <&crypto_mbox 16 0x1 0xffff>,
                     <&crypto_mbox 17 0x1 0xffff>,
                     <&crypto_mbox 30 0x1 0xffff>,
                     <&crypto_mbox 31 0x1 0xffff>;
};

The Crypto driver was recently submitted by Rob Rice.
Refer https://www.spinics.net/lists/kernel/msg2395306.html

Similar to Crypto offload engine, we also have RAID offload
engine. The RAID offload engine driver is also implemented
as mailbox client driver.

Regards,
Anup

^ permalink raw reply

* Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
From: Masahiro Yamada @ 2016-12-13  4:39 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-mmc, Adrian Hunter, Ulf Hansson, Douglas Anderson,
	devicetree, Al Cooper, Linux Kernel Mailing List, Stefan Wahren,
	Andrei Pistirica, Wolfram Sang, Mark Rutland, Simon Horman
In-Reply-To: <20161212171408.2yf4jauz7czv44fy@rob-hp-laptop>

Hi Rob.

2016-12-13 2:14 GMT+09:00 Rob Herring <robh@kernel.org>:
>> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
>> new file mode 100644
>> index 0000000..750374f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
>> @@ -0,0 +1,30 @@
>> +* Cadence SD/SDIO/eMMC Host Controller
>> +
>> +Required properties:
>> +- compatible: should be "cdns,sd4hc".
>
> Needs SoC specific compatible strings too.


I remember you mentioned the vendor prefix
stands for the SoC vendor, not the IP vendor.
The compatible prefixed with the IP vendor is prepared for a fallback.

I will add "socionext,sd4hc".




>> +- reg: offset and length of the register set for the device.
>> +- interrupts: a single interrupt specifier.
>> +- clocks: phandle to the input clock.
>> +
>> +Optional properties:
>> +For eMMC configuration, supported speed modes are not indicated by the SDHCI
>> +Capabilities Register.  Instead, the following properties should be specified
>> +if supported.  See mmc.txt for details.
>> +- mmc-ddr-1_8v
>> +- mmc-ddr-1_2v
>> +- mmc-hs200-1_8v
>> +- mmc-hs200-1_2v
>> +- mmc-hs400-1_8v
>> +- mmc-hs400-1_2v
>
> There's now a property to override SDHCI capabilities register. Maybe
> you should use that instead? I'll defer to Ulf.
>

I did not know this new property.

So, now we have two ways to specify MMC speed mode capabilities
by only touching DT.

[1] Add MMC mode flags directly, like I did.
[2] Use "sdhci-caps-mask" and "sdhci-caps"


The problem for [2] is that eMMC capabilities
do not perfectly correspond to the SDHCI capabilities register.


>> +- mmc-hs400-1_8v
>> +- mmc-hs400-1_2v

If the driver sets SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
we can use the bit63 of caps for specifying HS400.

But, this is not defined in the SDHCI standard.
#define  SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */



>> +- mmc-ddr-1_8v

For High Speed DDR, perhaps we can imply MMC_CAP_1_8V_DDR
from MMC_CAP_UHS_DDR50  (bit34 of caps)

This is not supported in the current code, but
if this is a good idea, I can send a patch.


>> +- mmc-ddr-1_2v

This does not have the corresponding bit, but
1.2V is not commonly used, so this is not a fatal problem.



What I can do at most now, is to delete the
Optional properties section entirely
so users can choose [1] or [2] as they like.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH v3 4/6] mfd: dt: Add bindings for the Aspeed LPC Host Controller (LHC)
From: Andrew Jeffery @ 2016-12-13  4:40 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lee Jones, Mark Rutland, Linus Walleij, Corey Minyard,
	Cédric Le Goater, Joel Stanley,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161212153018.lfkicwknikfbzdlf@rob-hp-laptop>

[-- Attachment #1: Type: text/plain, Size: 2357 bytes --]

On Mon, 2016-12-12 at 09:30 -0600, Rob Herring wrote:
> On Tue, Dec 06, 2016 at 01:53:19PM +1100, Andrew Jeffery wrote:
> > The LPC bus pinmux configuration on fifth generation Aspeed SoCs depends
> > on bits in both the System Control Unit and the LPC Host Controller.
> > 
> > The Aspeed LPC Host Controller is described as a child node of the
> > LPC host-range syscon device for arbitration of access by the host
> > controller and pinmux drivers.
> > 
> > > > Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
> > ---
> >  .../devicetree/bindings/mfd/aspeed-lpc.txt         | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> > index a97131aba446..9de318ef72da 100644
> > --- a/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> > +++ b/Documentation/devicetree/bindings/mfd/aspeed-lpc.txt
> > > > @@ -109,3 +109,25 @@ lpc: lpc@1e789000 {
> > > >  	};
> >  };
> >  
> > +Host Node Children
> > +==================
> > +
> > +LPC Host Controller
> > +-------------------
> > +
> > +The Aspeed LPC Host Controller configures the Low Pin Count (LPC) bus behaviour
> > +between the host and the baseboard management controller. The registers exist
> > +in the "host" portion of the Aspeed LPC controller, which must be the parent of
> > +the LPC host controller node.
> > +
> > +Required properties:
> > > > +- compatible:		"aspeed,ast2500-lhc";
> > > > +- reg:			contains offset/length value of the LHC memory
> > +			region.
> 
> How many regions? Looks like 2.

Yes, two. The first region is registers configuring various LPC host
controller properties. The second region is for configuring the LPC
serial IRQ trigger modes.

Would you like me to say as much in the patch? It's not a completely
clear split of functionality as other serial IRQ properties are also
described in the first region. Maybe describing the datasheet's
register names for the regions (LHCR[0-8] for the first, LHCR[A-B] for
the second) would help?

Cheers,

Andrew

> 
> > +
> > +Example:
> > +
> > > > +lhc: lhc@20 {
> > > > +	compatible = "aspeed,ast2500-lhc";
> > > > +	reg = <0x20 0x24 0x48 0x8>;
> > +};
> > -- 
> > 2.9.3
> > 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v3 6/6] mfd: dt: Move syscon bindings to syscon subdirectory
From: Andrew Jeffery @ 2016-12-13  4:53 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lee Jones, Mark Rutland, Linus Walleij, Corey Minyard,
	Cédric Le Goater, Joel Stanley, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <20161212153911.drhczoagyyhu7jm4@rob-hp-laptop>

[-- Attachment #1: Type: text/plain, Size: 2185 bytes --]

On Mon, 2016-12-12 at 09:39 -0600, Rob Herring wrote:
> On Tue, Dec 06, 2016 at 01:53:21PM +1100, Andrew Jeffery wrote:
> > The use of syscons is growing, lets collate them in their own part of
> > the bindings tree.
> > 
> > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> >  Documentation/devicetree/bindings/mfd/{ => syscon}/aspeed-scu.txt         | 0
> >  Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-gpbr.txt         | 0
> >  Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-matrix.txt       | 0
> >  Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-smc.txt          | 0
> >  Documentation/devicetree/bindings/mfd/{ => syscon}/qcom,tcsr.txt          | 0
> >  Documentation/devicetree/bindings/mfd/{ => syscon}/syscon.txt             | 0
> >  .../devicetree/bindings/mfd/{ => syscon}/ti-keystone-devctrl.txt          | 0
> >  7 files changed, 0 insertions(+), 0 deletions(-)
> >  rename Documentation/devicetree/bindings/mfd/{ => syscon}/aspeed-scu.txt (100%)
> >  rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-gpbr.txt (100%)
> >  rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-matrix.txt (100%)
> >  rename Documentation/devicetree/bindings/mfd/{ => syscon}/atmel-smc.txt (100%)
> >  rename Documentation/devicetree/bindings/mfd/{ => syscon}/qcom,tcsr.txt (100%)
> >  rename Documentation/devicetree/bindings/mfd/{ => syscon}/syscon.txt (100%)
> >  rename Documentation/devicetree/bindings/mfd/{ => syscon}/ti-keystone-devctrl.txt (100%)
> 
> I'm not so sure this is the right direction. syscon usage is pretty much 
> spread throughout the tree.

This patch was created based on my interpretation of Lee's feedback
here:

https://lkml.org/lkml/2016/11/18/650

Lee's next email in the chain poked Arnd for an opinion, but Arnd
didn't reply.

I don't mind. I moved these bindings separately so we could just drop
the patch if there was push-back. If we drop the whole idea I'll need
to apply a small fix to patch 5/6 to avoid creating the syscon
subdirectory.

Andrew

> 
> Rob

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v3 1/4] pinctrl: aspeed: Read and write bits in LPC and GFX controllers
From: Andrew Jeffery @ 2016-12-13  6:12 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Walleij, Mark Rutland, Joel Stanley, linux-gpio, devicetree,
	linux-kernel
In-Reply-To: <20161212162712.u2uypxkqhh6cinsp@rob-hp-laptop>

[-- Attachment #1: Type: text/plain, Size: 6466 bytes --]

On Mon, 2016-12-12 at 10:27 -0600, Rob Herring wrote:
> On Tue, Dec 06, 2016 at 02:11:49PM +1100, Andrew Jeffery wrote:
> > The System Control Unit IP block in the Aspeed SoCs is typically where
> > the pinmux configuration is found, but not always. A number of pins
> > depend on state in one of LPC Host Control (LHC) or SoC Display
> > Controller (GFX) IP blocks, so the Aspeed pinmux drivers should have the
> > means to adjust these as necessary.
> > 
> > We use syscon to cast a regmap over the GFX and LPC blocks, which is
> > used as an arbitration layer between the relevant driver and the pinctrl
> > subsystem. The regmaps are then exposed to the SoC-specific pinctrl
> > drivers by phandles in the devicetree, and are selected during a mux
> > request by querying a new 'ip' member in struct aspeed_sig_desc.
> > 
> > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> >  .../devicetree/bindings/pinctrl/pinctrl-aspeed.txt |  50 ++++++-
> >  drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c         |  18 +--
> >  drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c         |  48 ++++--
> >  drivers/pinctrl/aspeed/pinctrl-aspeed.c            | 161 +++++++++++++--------
> >  drivers/pinctrl/aspeed/pinctrl-aspeed.h            |  32 ++--
> >  5 files changed, 214 insertions(+), 95 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> > index 2ad18c4ea55c..115b0cce6c1c 100644
> > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt
> > @@ -4,12 +4,19 @@ Aspeed Pin Controllers
> >  The Aspeed SoCs vary in functionality inside a generation but have a common mux
> >  device register layout.
> >  
> > -Required properties:
> > -- compatible : Should be any one of the following:
> > > > -		"aspeed,ast2400-pinctrl"
> > > > -		"aspeed,g4-pinctrl"
> > > > -		"aspeed,ast2500-pinctrl"
> > > > -		"aspeed,g5-pinctrl"
> > +Required properties for g4:
> > > > +- compatible : 			Should be any one of the following:
> > > > +				"aspeed,ast2400-pinctrl"
> > > > +				"aspeed,g4-pinctrl"
> > +
> > +Required properties for g5:
> > > > +- compatible : 			Should be any one of the following:
> > > > +				"aspeed,ast2500-pinctrl"
> > > > +				"aspeed,g5-pinctrl"
> > +
> > > > +- aspeed,external-nodes:	A cell of phandles to external controller nodes:
> > > > +				0: compatible with "aspeed,ast2500-gfx", "syscon"
> > > > +				1: compatible with "aspeed,ast2500-lpchc", "syscon"
> >  
> >  The pin controller node should be a child of a syscon node with the required
> >  property:
> > @@ -47,7 +54,7 @@ RGMII1 RGMII2 RMII1 RMII2 SD1 SPI1 SPI1DEBUG SPI1PASSTHRU TIMER4 TIMER5 TIMER6
> >  TIMER7 TIMER8 VGABIOSROM
> >  
> >  
> > -Examples:
> > +g4 Example:
> >  
> > > >  syscon: scu@1e6e2000 {
> > > >  	compatible = "syscon", "simple-mfd";
> > > > @@ -63,5 +70,34 @@ syscon: scu@1e6e2000 {
> > > >  	};
> >  };
> >  
> > +g5 Example:
> > +
> > +apb {
> > > > > > +	gfx: display@1e6e6000 {
> > > > +		compatible = "aspeed,ast2500-gfx", "syscon";
> > > > +		reg = <0x1e6e6000 0x1000>;
> > > > +	};
> > +
> > > > > > +	lpchc: lpchc@1e7890a0 {
> > > > +		compatible = "aspeed,ast2500-lpchc", "syscon";
> > > > +		reg = <0x1e7890a0 0xc4>;
> > > > +	};
> > +
> > > > > > +	syscon: scu@1e6e2000 {
> > +		compatible = "syscon", "simple-mfd";
> 
> I must have missed this the first time, but "syscon" should be used with 
> a specific compatible. Though, the scu binding does define one.

Yes, the example should be fixed.

> 
> > > > +		reg = <0x1e6e2000 0x1a8>;
> > +
> > +		pinctrl: pinctrl {
> 
> Is this the only child? 

No. A incomplete list of other functions in the SCU includes:

* An RNG
* Power management
* PCI configuration
* System reset
* Clock configuration

> 
> > +			compatible = "aspeed,g5-pinctrl";
> 
> There's no register range for pinctrl?

This may be a mistake on my part; when I wrote this I had no experience
with writing devicetree bindings (and still don't have a lot).

The SCU does have register regions for pinctrl but on reflection I feel
neither the mfd nor syscon bindings describe how children's resources
should be treated in general. The example in the mfd bindings is for
hardware that is register-bit-led,compatible, whose bindings use the
'offset' property rather than 'reg', which still describes where, but
not using the reg property. Given my uncertainty with reg in an mfd
child, I wrote the pinctrl/pinmux driver using offsets from the base of
the SCU's syscon rather than describing the exact region(s) of the
syscon that should be used.

The issue you raise here occurred to me when writing the LPC Host
Controller bindings, but there I wasn't convinced using the ranges
property to give offsets was the right thing to do either.

Regardless, whilst there are two dedicated regions of pinmux registers,
the mux state also depends on bits in SCU registers outside of these
regions. Assuming we define an appropriate ranges property for the SCU
syscon the pinctrl reg property would look like:

    reg = <0x2c 0x1>, <0x3c 0x1>, <0x48 0x1>, <0x70 0x1>, <0x7c 0x1>, <0x80 0x18>, <0xa0 0x10>, <0xd0 0x1>;

This is the list of registers affecting the mux taken from the pinctrl-
aspeed.h.

What action do you recommend here? The pinctrl dts patches for the
Aspeed SoCs are yet to be applied, so changing the bindings to require
a reg property can't break any existing in-tree users as there are
none. The pinctrl driver can be patched to respect the reg property
after the fact, though actually using the region descriptions might be
interesting.

> 
> > +			aspeed,external-nodes = <&gfx, &lpchc>;

Did you have feedback on this approach? I queried you about it in the
previous revision, but never received a reply:

https://marc.info/?l=devicetree&m=147873554311535&w=4

Thanks,

Andrew

> > +
> > > > +			pinctrl_i2c3_default: i2c3_default {
> > > > +				function = "I2C3";
> > > > +				groups = "I2C3";
> > > > +			};
> > > > +		};
> > > > +	};
> > +};
> > +
> >  Please refer to pinctrl-bindings.txt in this directory for details of the
> >  common pinctrl bindings used by client devices.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH v4 1/4] mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
From: Marek Vasut @ 2016-12-13  7:50 UTC (permalink / raw)
  To: Cédric Le Goater, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: David Woodhouse, Brian Norris, Boris Brezillon,
	Richard Weinberger, Cyrille Pitchen,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Joel Stanley
In-Reply-To: <1481557252-13656-2-git-send-email-clg-Bxea+6Xhats@public.gmane.org>

On 12/12/2016 04:40 PM, Cédric Le Goater wrote:
> This driver adds mtd support for the Aspeed AST2500 SoC static memory
> controllers :

[...]

> +#define DEVICE_NAME	"aspeed-smc"
> +
> +/*
> + * The driver only support SPI flash
> + */
> +enum aspeed_smc_flash_type {
> +	smc_type_nor  = 0,
> +	smc_type_nand = 1,
> +	smc_type_spi  = 2,
> +};

So why is this here ? :)

> +struct aspeed_smc_chip;
> +
> +struct aspeed_smc_info {
> +	u32 maxsize;		/* maximum size of chip window */
> +	u8 nce;			/* number of chip enables */
> +	bool hastype;		/* flash type field exists in config reg */
> +	u8 we0;			/* shift for write enable bit for CE0 */
> +	u8 ctl0;		/* offset in regs of ctl for CE0 */
> +
> +	void (*set_4b)(struct aspeed_smc_chip *chip);
> +};

Otherwise looks good:
Reviewed-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

-- 
Best regards,
Marek Vasut
--
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 v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
From: Ulf Hansson @ 2016-12-13  7:52 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Rob Herring, linux-mmc, Adrian Hunter, Douglas Anderson,
	devicetree@vger.kernel.org, Al Cooper, Linux Kernel Mailing List,
	Stefan Wahren, Andrei Pistirica, Wolfram Sang, Mark Rutland,
	Simon Horman
In-Reply-To: <CAK7LNATO+vpxmDV-XEupjeXXhPNdrXHNwfwWcAVWLdbJi+37QA@mail.gmail.com>

[...]

>>> +
>>> +Optional properties:
>>> +For eMMC configuration, supported speed modes are not indicated by the SDHCI
>>> +Capabilities Register.  Instead, the following properties should be specified
>>> +if supported.  See mmc.txt for details.
>>> +- mmc-ddr-1_8v
>>> +- mmc-ddr-1_2v
>>> +- mmc-hs200-1_8v
>>> +- mmc-hs200-1_2v
>>> +- mmc-hs400-1_8v
>>> +- mmc-hs400-1_2v
>>
>> There's now a property to override SDHCI capabilities register. Maybe
>> you should use that instead? I'll defer to Ulf.
>>
>
> I did not know this new property.
>
> So, now we have two ways to specify MMC speed mode capabilities
> by only touching DT.

Let me clarify a bit.

The point with the new bindings is to be able to override *broken*
SDHCI caps bits. So, not only related to the speed modes.

>
> [1] Add MMC mode flags directly, like I did.
> [2] Use "sdhci-caps-mask" and "sdhci-caps"
>
>
> The problem for [2] is that eMMC capabilities
> do not perfectly correspond to the SDHCI capabilities register.
>
>
>>> +- mmc-hs400-1_8v
>>> +- mmc-hs400-1_2v
>
> If the driver sets SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
> we can use the bit63 of caps for specifying HS400.
>
> But, this is not defined in the SDHCI standard.
> #define  SDHCI_SUPPORT_HS400 0x80000000 /* Non-standard */
>
>
>
>>> +- mmc-ddr-1_8v
>
> For High Speed DDR, perhaps we can imply MMC_CAP_1_8V_DDR
> from MMC_CAP_UHS_DDR50  (bit34 of caps)
>
> This is not supported in the current code, but
> if this is a good idea, I can send a patch.
>
>
>>> +- mmc-ddr-1_2v
>
> This does not have the corresponding bit, but
> 1.2V is not commonly used, so this is not a fatal problem.
>
>
>
> What I can do at most now, is to delete the
> Optional properties section entirely
> so users can choose [1] or [2] as they like.
>

I think a better approach is to use the new sdhci-caps* bindings to
mask those caps that can't be trusted. And then use the generic mmc
bindings for speed modes instead.

That should be a safer approach, right!?

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH 4/4] mmc: pwrseq-simple: add disable-post-power-on option
From: Ulf Hansson @ 2016-12-13  8:01 UTC (permalink / raw)
  To: Rob Herring, Matt Ranostay
  Cc: devicetree@vger.kernel.org, linux-mmc@vger.kernel.org,
	Tony Lindgren, Liam Breck
In-Reply-To: <20161209180930.nsy2ys66q2thbxpx@rob-hp-laptop>

On 9 December 2016 at 19:09, Rob Herring <robh@kernel.org> wrote:
> On Fri, Dec 02, 2016 at 01:06:47AM -0800, Matt Ranostay wrote:
>> On Fri, Dec 2, 2016 at 12:28 AM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> > On 2 December 2016 at 08:22, Matt Ranostay <matt@ranostay.consulting> wrote:
>> >>
>> >>
>> >>> On Dec 1, 2016, at 23:13, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> >>>
>> >>>> On 2 December 2016 at 07:17, Matt Ranostay <matt@ranostay.consulting> wrote:
>> >>>> Add optional device-post-power-on parameters to disable post_power_on
>> >>>> function from being called since this breaks some device.
>> >>>>
>> >>>> Cc: Tony Lindgren <tony@atomide.com>
>> >>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> >>>> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>
>> >>>> ---
>> >>>> Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt | 2 ++
>> >>>> drivers/mmc/core/pwrseq_simple.c                            | 7 +++++++
>> >>>> 2 files changed, 9 insertions(+)
>> >>>>
>> >>>> diff --git a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
>> >>>> index bea306d772d1..09fa153f743e 100644
>> >>>> --- a/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
>> >>>> +++ b/Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt
>> >>>> @@ -24,6 +24,8 @@ Optional properties:
>> >>>>        de-asserting the reset-gpios (if any)
>> >>>> - invert-off-state: Invert the power down state for the reset-gpios (if any)
>> >>>>        and pwrdn-gpios (if any)
>> >>>> +- disable-post-power-on : Avoid post_power_on function from being called since
>> >>>> +       this breaks some devices
>> >>>
>> >>> This is a bit weird. I would like to avoid this, if possible.
>> >>>
>> >>> Perhaps if you simply describe the sequence in detail for your device
>> >>> we can figure out the best option together.
>> >>
>> >> Yeah I know it is a bit weird but we need to keep that reset pin high for at least some time after pre power on.   So any case it would be another property
>> >
>> > This went offlist, please resend.
>> >
>> > Moreover, please try to describe the sequence you need in detail
>> > according to your HW spec.
>> >
>>
>> Yeah oops....
>>
>> So basically we need to drive the reset and powerdown lines high with
>> a 300 milliseconds delay between both...
>> can't have the reset line low with post power on (pretty sure but
>> would need a delay anyway), and we need both reset + powerdown line
>> set low on powerdown.
>>
>> So the power down sequence would need to be reversed for this
>> application in pwrseq-simple.
>
> This sounds like you need a device specific sequence to me. Otherwise,
> write a language to describe any power control waveforms rather than
> trying to bolt on more and more properties. (Don't really go write a
> language.)

Actually this isn't so device specific. The cw1200 wifi chip which is
being used with ux500 SoCs has a very similar sequence. Allow me to
check the details and get back.

Anyway, my point is that I think we can figure out a common sequence
to support these kind required sequences. That is to me preferred over
adding a new (two to support cw1200) device specific pwrseq driver.

Kind regards
Uffe

^ permalink raw reply

* Re: [PATCH v6 4/5] ARM: dts: da850-lcdk: add the vga-bridge node
From: Tomi Valkeinen @ 2016-12-13  8:46 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Mark Rutland, linux-devicetree, linux-drm, Kevin Hilman,
	Michael Turquette, Sekhar Nori, Russell King, Jyri Sarha, LKML,
	Peter Ujfalusi, Rob Herring, Laurent Pinchart, Frank Rowand,
	arm-soc
In-Reply-To: <1481547942-24775-5-git-send-email-bgolaszewski@baylibre.com>


[-- Attachment #1.1.1: Type: text/plain, Size: 877 bytes --]

Hi,

On 12/12/16 15:05, Bartosz Golaszewski wrote:

> +&lcdc {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&lcd_pins>;
> +
> +	ports {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		lcdc_out: port@1 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <1>;
> +
> +			lcdc_out_vga: endpoint {
> +				reg = <0>;
> +				remote-endpoint = <&vga_bridge_in>;
> +			};
> +		};
> +	};
> +};
> 

This is not correct. LCDC has just one output, so port@1 doesn't make
sense. It's port@0. But with just one port, you can leave "ports" away.
And you don't need the port's label for anything, if I'm not mistaken. So:

&lcdc {
	status = "okay";
	pinctrl-names = "default";
	pinctrl-0 = <&lcd_pins>;

	port {
		lcdc_out_vga: endpoint {
			remote-endpoint = <&vga_bridge_in>;
		};
	};
};

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH v1 0/3] clk: rockchip: support clk controller for rk3328 SoC
From: Elaine Zhang @ 2016-12-13  8:47 UTC (permalink / raw)
  To: heiko, mturquette, sboyd, xf
  Cc: robh+dt, mark.rutland, linux-clk, linux-arm-kernel, devicetree,
	huangtao, xxx, cl, linux-rockchip, linux-kernel, Elaine Zhang

Elaine Zhang (3):
  clk: rockchip: add dt-binding header for rk3328
  clk: rockchip: add clock controller for rk3328
  clk: rockchip: add new pll-type for rk3328 and similar socs

 drivers/clk/rockchip/Makefile          |    1 +
 drivers/clk/rockchip/clk-pll.c         |   13 +-
 drivers/clk/rockchip/clk-rk3328.c      | 1068 ++++++++++++++++++++++++++++++++
 drivers/clk/rockchip/clk.h             |   23 +
 include/dt-bindings/clock/rk3328-cru.h |  403 ++++++++++++
 5 files changed, 1507 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/rockchip/clk-rk3328.c
 create mode 100644 include/dt-bindings/clock/rk3328-cru.h

-- 
1.9.1

^ permalink raw reply

* [PATCH v1 1/3] clk: rockchip: add dt-binding header for rk3328
From: Elaine Zhang @ 2016-12-13  8:47 UTC (permalink / raw)
  To: heiko, mturquette, sboyd, xf
  Cc: robh+dt, mark.rutland, linux-clk, linux-arm-kernel, devicetree,
	huangtao, xxx, cl, linux-rockchip, linux-kernel, Elaine Zhang
In-Reply-To: <1481618869-1239-1-git-send-email-zhangqing@rock-chips.com>

Add the dt-bindings header for the rk3328, that gets shared between
the clock controller and the clock references in the dts.
Add softreset ID for rk3328.

Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
 include/dt-bindings/clock/rk3328-cru.h | 403 +++++++++++++++++++++++++++++++++
 1 file changed, 403 insertions(+)
 create mode 100644 include/dt-bindings/clock/rk3328-cru.h

diff --git a/include/dt-bindings/clock/rk3328-cru.h b/include/dt-bindings/clock/rk3328-cru.h
new file mode 100644
index 000000000000..545ed7541316
--- /dev/null
+++ b/include/dt-bindings/clock/rk3328-cru.h
@@ -0,0 +1,403 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Elaine <zhangqing@rock-chips.com>
+ *
+ * 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.
+ */
+
+#ifndef _DT_BINDINGS_CLK_ROCKCHIP_RK3328_H
+#define _DT_BINDINGS_CLK_ROCKCHIP_RK3328_H
+
+/* core clocks */
+#define PLL_APLL		1
+#define PLL_DPLL		2
+#define PLL_CPLL		3
+#define PLL_GPLL		4
+#define PLL_NPLL		5
+#define ARMCLK			6
+
+/* sclk gates (special clocks) */
+#define SCLK_RTC32K		30
+#define SCLK_SDMMC_EXT		31
+#define SCLK_SPI		32
+#define SCLK_SDMMC		33
+#define SCLK_SDIO		34
+#define SCLK_EMMC		35
+#define SCLK_TSADC		36
+#define SCLK_SARADC		37
+#define SCLK_UART0		38
+#define SCLK_UART1		39
+#define SCLK_UART2		40
+#define SCLK_I2S0		41
+#define SCLK_I2S1		42
+#define SCLK_I2S2		43
+#define SCLK_I2S1_OUT		44
+#define SCLK_I2S2_OUT		45
+#define SCLK_SPDIF		46
+#define SCLK_TIMER0		47
+#define SCLK_TIMER1		48
+#define SCLK_TIMER2		49
+#define SCLK_TIMER3		50
+#define SCLK_TIMER4		51
+#define SCLK_TIMER5		52
+#define SCLK_WIFI		53
+#define SCLK_CIF_OUT		54
+#define SCLK_I2C0		55
+#define SCLK_I2C1		56
+#define SCLK_I2C2		57
+#define SCLK_I2C3		58
+#define SCLK_CRYPTO		59
+#define SCLK_PWM		60
+#define SCLK_PDM		61
+#define SCLK_EFUSE		62
+#define SCLK_OTP		63
+#define SCLK_DDRCLK		64
+#define SCLK_VDEC_CABAC		65
+#define SCLK_VDEC_CORE		66
+#define SCLK_VENC_DSP		67
+#define SCLK_VENC_CORE		68
+#define SCLK_RGA		69
+#define SCLK_HDMI_SFC		70
+#define SCLK_HDMI_CEC		71
+#define SCLK_USB3_REF		72
+#define SCLK_USB3_SUSPEND	73
+#define SCLK_SDMMC_DRV		74
+#define SCLK_SDIO_DRV		75
+#define SCLK_EMMC_DRV		76
+#define SCLK_SDMMC_EXT_DRV	77
+#define SCLK_SDMMC_SAMPLE	78
+#define SCLK_SDIO_SAMPLE	79
+#define SCLK_EMMC_SAMPLE	80
+#define SCLK_SDMMC_EXT_SAMPLE	81
+#define SCLK_VOP		82
+#define SCLK_MAC2PHY_RXTX	83
+#define SCLK_MAC2PHY_SRC	84
+#define SCLK_MAC2PHY_REF	85
+#define SCLK_MAC2PHY_OUT	86
+#define SCLK_MAC2IO_RX		87
+#define SCLK_MAC2IO_TX		88
+#define SCLK_MAC2IO_REFOUT	89
+#define SCLK_MAC2IO_REF		90
+#define SCLK_MAC2IO_OUT		91
+#define SCLK_TSP		92
+#define SCLK_HSADC_TSP		93
+#define SCLK_USB3PHY_REF	94
+#define SCLK_REF_USB3OTG	95
+#define SCLK_USB3OTG_REF	96
+#define SCLK_USB3OTG_SUSPEND	97
+#define SCLK_REF_USB3OTG_SRC	98
+#define SCLK_MAC2IO_SRC		99
+
+/* dclk gates */
+#define DCLK_LCDC		180
+#define DCLK_HDMIPHY		181
+#define HDMIPHY			182
+#define USB480M			183
+#define DCLK_LCDC_SRC		184
+
+/* aclk gates */
+#define ACLK_AXISRAM		190
+#define ACLK_VOP_PRE		191
+#define ACLK_USB3OTG		192
+#define ACLK_RGA_PRE		193
+#define ACLK_DMAC		194
+#define ACLK_GPU		195
+#define ACLK_BUS_PRE		196
+#define ACLK_PERI_PRE		197
+#define ACLK_RKVDEC_PRE		198
+#define ACLK_RKVDEC		199
+#define ACLK_RKVENC		200
+#define ACLK_VPU_PRE		201
+#define ACLK_VIO_PRE		202
+#define ACLK_VPU		203
+#define ACLK_VIO		204
+#define ACLK_VOP		205
+#define ACLK_GMAC		206
+#define ACLK_H265		207
+#define ACLK_H264		208
+#define ACLK_MAC2PHY		209
+#define ACLK_MAC2IO		210
+#define ACLK_DCF		211
+#define ACLK_TSP		212
+#define ACLK_PERI		213
+#define ACLK_RGA		214
+#define ACLK_IEP		215
+#define ACLK_CIF		216
+#define ACLK_HDCP		217
+
+/* pclk gates */
+#define PCLK_GPIO0		300
+#define PCLK_GPIO1		301
+#define PCLK_GPIO2		302
+#define PCLK_GPIO3		303
+#define PCLK_GRF		304
+#define PCLK_I2C0		305
+#define PCLK_I2C1		306
+#define PCLK_I2C2		307
+#define PCLK_I2C3		308
+#define PCLK_SPI		309
+#define PCLK_UART0		310
+#define PCLK_UART1		311
+#define PCLK_UART2		312
+#define PCLK_TSADC		313
+#define PCLK_PWM		314
+#define PCLK_TIMER		315
+#define PCLK_BUS_PRE		316
+#define PCLK_PERI_PRE		317
+#define PCLK_HDMI_CTRL		318
+#define PCLK_HDMI_PHY		319
+#define PCLK_GMAC		320
+#define PCLK_H265		321
+#define PCLK_MAC2PHY		322
+#define PCLK_MAC2IO		323
+#define PCLK_USB3PHY_OTG	324
+#define PCLK_USB3PHY_PIPE	325
+#define PCLK_USB3_GRF		326
+#define PCLK_USB2_GRF		327
+#define PCLK_HDMIPHY		328
+#define PCLK_DDR		329
+#define PCLK_PERI		330
+#define PCLK_HDMI		331
+#define PCLK_HDCP		332
+#define PCLK_DCF		333
+#define PCLK_SARADC		334
+
+/* hclk gates */
+#define HCLK_PERI		408
+#define HCLK_TSP		409
+#define HCLK_GMAC		410
+#define HCLK_I2S0_8CH		411
+#define HCLK_I2S1_8CH		413
+#define HCLK_I2S2_2CH		413
+#define HCLK_SPDIF_8CH		414
+#define HCLK_VOP		415
+#define HCLK_NANDC		416
+#define HCLK_SDMMC		417
+#define HCLK_SDIO		418
+#define HCLK_EMMC		419
+#define HCLK_SDMMC_EXT		420
+#define HCLK_RKVDEC_PRE		421
+#define HCLK_RKVDEC		422
+#define HCLK_RKVENC		423
+#define HCLK_VPU_PRE		424
+#define HCLK_VIO_PRE		425
+#define HCLK_VPU		426
+#define HCLK_VIO		427
+#define HCLK_BUS_PRE		428
+#define HCLK_PERI_PRE		429
+#define HCLK_H264		430
+#define HCLK_CIF		431
+#define HCLK_OTG_PMU		432
+#define HCLK_OTG		433
+#define HCLK_HOST0		434
+#define HCLK_HOST0_ARB		435
+#define HCLK_CRYPTO_MST		436
+#define HCLK_CRYPTO_SLV		437
+#define HCLK_PDM		438
+#define HCLK_IEP		439
+#define HCLK_RGA		440
+#define HCLK_HDCP		441
+
+#define CLK_NR_CLKS		(HCLK_HDCP + 1)
+
+#define SCLK_MAC2IO		0
+#define SCLK_MAC2PHY		1
+
+#define CLKGRF_NR_CLKS		(SCLK_MAC2PHY + 1)
+
+/* soft-reset indices */
+#define SRST_CORE0_PO		0
+#define SRST_CORE1_PO		1
+#define SRST_CORE2_PO		2
+#define SRST_CORE3_PO		3
+#define SRST_CORE0		4
+#define SRST_CORE1		5
+#define SRST_CORE2		6
+#define SRST_CORE3		7
+#define SRST_CORE0_DBG		8
+#define SRST_CORE1_DBG		9
+#define SRST_CORE2_DBG		10
+#define SRST_CORE3_DBG		11
+#define SRST_TOPDBG		12
+#define SRST_CORE_NIU		13
+#define SRST_STRC_A		14
+#define SRST_L2C		15
+
+#define SRST_A53_GIC		18
+#define SRST_DAP		19
+#define SRST_PMU_P		21
+#define SRST_EFUSE		22
+#define SRST_BUSSYS_H		23
+#define SRST_BUSSYS_P		24
+#define SRST_SPDIF		25
+#define SRST_INTMEM		26
+#define SRST_ROM		27
+#define SRST_GPIO0		28
+#define SRST_GPIO1		29
+#define SRST_GPIO2		30
+#define SRST_GPIO3		31
+
+#define SRST_I2S0		32
+#define SRST_I2S1		33
+#define SRST_I2S2		34
+#define SRST_I2S0_H		35
+#define SRST_I2S1_H		36
+#define SRST_I2S2_H		37
+#define SRST_UART0		38
+#define SRST_UART1		39
+#define SRST_UART2		40
+#define SRST_UART0_P		41
+#define SRST_UART1_P		42
+#define SRST_UART2_P		43
+#define SRST_I2C0		44
+#define SRST_I2C1		45
+#define SRST_I2C2		46
+#define SRST_I2C3		47
+
+#define SRST_I2C0_P		48
+#define SRST_I2C1_P		49
+#define SRST_I2C2_P		50
+#define SRST_I2C3_P		51
+#define SRST_EFUSE_SE_P		52
+#define SRST_EFUSE_NS_P		53
+#define SRST_PWM0		54
+#define SRST_PWM0_P		55
+#define SRST_DMA		56
+#define SRST_TSP_A		57
+#define SRST_TSP_H		58
+#define SRST_TSP		59
+#define SRST_TSP_HSADC		60
+#define SRST_DCF_A		61
+#define SRST_DCF_P		62
+
+#define SRST_SCR		64
+#define SRST_SPI		65
+#define SRST_TSADC		66
+#define SRST_TSADC_P		67
+#define SRST_CRYPTO		68
+#define SRST_SGRF		69
+#define SRST_GRF		70
+#define SRST_USB_GRF		71
+#define SRST_TIMER_6CH_P	72
+#define SRST_TIMER0		73
+#define SRST_TIMER1		74
+#define SRST_TIMER2		75
+#define SRST_TIMER3		76
+#define SRST_TIMER4		77
+#define SRST_TIMER5		78
+#define SRST_USB3GRF		79
+
+#define SRST_PHYNIU		80
+#define SRST_HDMIPHY		81
+#define SRST_VDAC		82
+#define SRST_ACODEC_p		83
+#define SRST_SARADC		85
+#define SRST_SARADC_P		86
+#define SRST_GRF_DDR		87
+#define SRST_DFIMON		88
+#define SRST_MSCH		89
+#define SRST_DDRMSCH		91
+#define SRST_DDRCTRL		92
+#define SRST_DDRCTRL_P		93
+#define SRST_DDRPHY		94
+#define SRST_DDRPHY_P		95
+
+#define SRST_GMAC_NIU_A		96
+#define SRST_GMAC_NIU_P		97
+#define SRST_GMAC2PHY_A		98
+#define SRST_GMAC2IO_A		99
+#define SRST_MACPHY		100
+#define SRST_OTP_PHY		101
+#define SRST_GPU_A		102
+#define SRST_GPU_NIU_A		103
+#define SRST_SDMMCEXT		104
+#define SRST_PERIPH_NIU_A	105
+#define SRST_PERIHP_NIU_H	106
+#define SRST_PERIHP_P		107
+#define SRST_PERIPHSYS_H	108
+#define SRST_MMC0		109
+#define SRST_SDIO		110
+#define SRST_EMMC		111
+
+#define SRST_USB2OTG_H		112
+#define SRST_USB2OTG		113
+#define SRST_USB2OTG_ADP	114
+#define SRST_USB2HOST_H		115
+#define SRST_USB2HOST_ARB	116
+#define SRST_USB2HOST_AUX	117
+#define SRST_USB2HOST_EHCIPHY	118
+#define SRST_USB2HOST_UTMI	119
+#define SRST_USB3OTG		120
+#define SRST_USBPOR		121
+#define SRST_USB2OTG_UTMI	122
+#define SRST_USB2HOST_PHY_UTMI	123
+#define SRST_USB3OTG_UTMI	124
+#define SRST_USB3PHY_U2		125
+#define SRST_USB3PHY_U3		126
+#define SRST_USB3PHY_PIPE	127
+
+#define SRST_VIO_A		128
+#define SRST_VIO_BUS_H		129
+#define SRST_VIO_H2P_H		130
+#define SRST_VIO_ARBI_H		131
+#define SRST_VOP_NIU_A		132
+#define SRST_VOP_A		133
+#define SRST_VOP_H		134
+#define SRST_VOP_D		135
+#define SRST_RGA		136
+#define SRST_RGA_NIU_A		137
+#define SRST_RGA_A		138
+#define SRST_RGA_H		139
+#define SRST_IEP_A		140
+#define SRST_IEP_H		141
+#define SRST_HDMI		142
+#define SRST_HDMI_P		143
+
+#define SRST_HDCP_A		144
+#define SRST_HDCP		145
+#define SRST_HDCP_H		146
+#define SRST_CIF_A		147
+#define SRST_CIF_H		148
+#define SRST_CIF_P		149
+#define SRST_OTP_P		150
+#define SRST_OTP_SBPI		151
+#define SRST_OTP_USER		152
+#define SRST_DDRCTRL_A		153
+#define SRST_DDRSTDY_P		154
+#define SRST_DDRSTDY		155
+#define SRST_PDM_H		156
+#define SRST_PDM		157
+#define SRST_USB3PHY_OTG_P	158
+#define SRST_USB3PHY_PIPE_P	159
+
+#define SRST_VCODEC_A		160
+#define SRST_VCODEC_NIU_A	161
+#define SRST_VCODEC_H		162
+#define SRST_VCODEC_NIU_H	163
+#define SRST_VDEC_A		164
+#define SRST_VDEC_NIU_A		165
+#define SRST_VDEC_H		166
+#define SRST_VDEC_NIU_H		167
+#define SRST_VDEC_CORE		168
+#define SRST_VDEC_CABAC		169
+#define SRST_DDRPHYDIV		175
+
+#define SRST_RKVENC_NIU_A	176
+#define SRST_RKVENC_NIU_H	177
+#define SRST_RKVENC_H265_A	178
+#define SRST_RKVENC_H265_P	179
+#define SRST_RKVENC_H265_CORE	180
+#define SRST_RKVENC_H265_DSP	181
+#define SRST_RKVENC_H264_A	182
+#define SRST_RKVENC_H264_H	183
+#define SRST_RKVENC_INTMEM	184
+
+#endif
-- 
1.9.1



^ permalink raw reply related

* [PATCH v1 2/3] clk: rockchip: add clock controller for rk3328
From: Elaine Zhang @ 2016-12-13  8:47 UTC (permalink / raw)
  To: heiko-4mtYJXux2i+zQB+pC5nmwQ, mturquette-rdvid1DuHRBWk0Htik3J/w,
	sboyd-sgV2jX0FEOL9JmXXK+q4OQ, xf-TNX95d0MmH7DzftRWevZcw
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	huangtao-TNX95d0MmH7DzftRWevZcw, xxx-TNX95d0MmH7DzftRWevZcw,
	cl-TNX95d0MmH7DzftRWevZcw,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Elaine Zhang
In-Reply-To: <1481618869-1239-1-git-send-email-zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

Add the clock tree definition for the new rk3328 SoC.

Signed-off-by: Elaine Zhang <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
 drivers/clk/rockchip/Makefile     |    1 +
 drivers/clk/rockchip/clk-rk3328.c | 1068 +++++++++++++++++++++++++++++++++++++
 drivers/clk/rockchip/clk.h        |   23 +
 3 files changed, 1092 insertions(+)
 create mode 100644 drivers/clk/rockchip/clk-rk3328.c

diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile
index 16e098c36f90..68b04bfca282 100644
--- a/drivers/clk/rockchip/Makefile
+++ b/drivers/clk/rockchip/Makefile
@@ -16,5 +16,6 @@ obj-y	+= clk-rk3036.o
 obj-y	+= clk-rk3188.o
 obj-y	+= clk-rk3228.o
 obj-y	+= clk-rk3288.o
+obj-y	+= clk-rk3328.o
 obj-y	+= clk-rk3368.o
 obj-y	+= clk-rk3399.o
diff --git a/drivers/clk/rockchip/clk-rk3328.c b/drivers/clk/rockchip/clk-rk3328.c
new file mode 100644
index 000000000000..0a04d8ab4d61
--- /dev/null
+++ b/drivers/clk/rockchip/clk-rk3328.c
@@ -0,0 +1,1068 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Elaine <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ *
+ * 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.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/syscore_ops.h>
+#include <dt-bindings/clock/rk3328-cru.h>
+#include "clk.h"
+
+#define RK3328_GRF_SOC_STATUS0		0x480
+#define RK3328_GRF_MAC_CON1		0x904
+#define RK3328_GRF_MAC_CON2		0x908
+
+enum rk3328_plls {
+	apll, dpll, cpll, gpll, npll,
+};
+
+static struct rockchip_pll_rate_table rk3328_pll_rates[] = {
+	/* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */
+	RK3036_PLL_RATE(1608000000, 1, 67, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1584000000, 1, 66, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1560000000, 1, 65, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1536000000, 1, 64, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1512000000, 1, 63, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1488000000, 1, 62, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1464000000, 1, 61, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1440000000, 1, 60, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1416000000, 1, 59, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1392000000, 1, 58, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1368000000, 1, 57, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1344000000, 1, 56, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1320000000, 1, 55, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1296000000, 1, 54, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1272000000, 1, 53, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1248000000, 1, 52, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1200000000, 1, 50, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1188000000, 2, 99, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1104000000, 1, 46, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1100000000, 12, 550, 1, 1, 1, 0),
+	RK3036_PLL_RATE(1008000000, 1, 84, 2, 1, 1, 0),
+	RK3036_PLL_RATE(1000000000, 6, 500, 2, 1, 1, 0),
+	RK3036_PLL_RATE(984000000, 1, 82, 2, 1, 1, 0),
+	RK3036_PLL_RATE(960000000, 1, 80, 2, 1, 1, 0),
+	RK3036_PLL_RATE(936000000, 1, 78, 2, 1, 1, 0),
+	RK3036_PLL_RATE(912000000, 1, 76, 2, 1, 1, 0),
+	RK3036_PLL_RATE(900000000, 4, 300, 2, 1, 1, 0),
+	RK3036_PLL_RATE(888000000, 1, 74, 2, 1, 1, 0),
+	RK3036_PLL_RATE(864000000, 1, 72, 2, 1, 1, 0),
+	RK3036_PLL_RATE(840000000, 1, 70, 2, 1, 1, 0),
+	RK3036_PLL_RATE(816000000, 1, 68, 2, 1, 1, 0),
+	RK3036_PLL_RATE(800000000, 6, 400, 2, 1, 1, 0),
+	RK3036_PLL_RATE(700000000, 6, 350, 2, 1, 1, 0),
+	RK3036_PLL_RATE(696000000, 1, 58, 2, 1, 1, 0),
+	RK3036_PLL_RATE(600000000, 1, 75, 3, 1, 1, 0),
+	RK3036_PLL_RATE(594000000, 2, 99, 2, 1, 1, 0),
+	RK3036_PLL_RATE(504000000, 1, 63, 3, 1, 1, 0),
+	RK3036_PLL_RATE(500000000, 6, 250, 2, 1, 1, 0),
+	RK3036_PLL_RATE(408000000, 1, 68, 2, 2, 1, 0),
+	RK3036_PLL_RATE(312000000, 1, 52, 2, 2, 1, 0),
+	RK3036_PLL_RATE(216000000, 1, 72, 4, 2, 1, 0),
+	RK3036_PLL_RATE(96000000, 1, 64, 4, 4, 1, 0),
+	{ /* sentinel */ },
+};
+
+static struct rockchip_pll_rate_table rk3328_pll_frac_rates[] = {
+	/* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */
+	RK3036_PLL_RATE(1016064000, 3, 127, 1, 1, 0, 134217),
+	/* vco = 1016064000 */
+	RK3036_PLL_RATE(983040000, 24, 983, 1, 1, 0, 671088),
+	/* vco = 983040000 */
+	RK3036_PLL_RATE(491520000, 24, 983, 2, 1, 0, 671088),
+	/* vco = 983040000 */
+	RK3036_PLL_RATE(61440000, 6, 215, 7, 2, 0, 671088),
+	/* vco = 860156000 */
+	RK3036_PLL_RATE(56448000, 12, 451, 4, 4, 0, 9797894),
+	/* vco = 903168000 */
+	RK3036_PLL_RATE(40960000, 12, 409, 4, 5, 0, 10066329),
+	/* vco = 819200000 */
+	{ /* sentinel */ },
+};
+
+#define RK3328_DIV_CPU_MASK		0x1f
+#define RK3328_DIV_CPU_SHIFT		8
+
+#define RK3328_DIV_PERI_MASK		0xf
+#define RK3328_DIV_PERI_SHIFT		0
+#define RK3328_DIV_ACLK_MASK		0x7
+#define RK3328_DIV_ACLK_SHIFT		4
+#define RK3328_DIV_HCLK_MASK		0x3
+#define RK3328_DIV_HCLK_SHIFT		8
+#define RK3328_DIV_PCLK_MASK		0x7
+#define RK3328_DIV_PCLK_SHIFT		12
+
+#define RK3328_CLKSEL1(_aclk_core, _pclk_dbg)				\
+{									\
+	.reg = RK3328_CLKSEL_CON(1),					\
+	.val = HIWORD_UPDATE(_aclk_core, RK3328_DIV_ACLKM_MASK,		\
+			     RK3328_DIV_ACLKM_SHIFT) |			\
+	       HIWORD_UPDATE(_pclk_dbg, RK3328_DIV_PCLK_DBG_MASK,	\
+			     RK3328_DIV_PCLK_DBG_SHIFT),		\
+}
+
+#define RK3328_CPUCLK_RATE(_prate, _aclk_core, _pclk_dbg)		\
+{									\
+	.prate = _prate,						\
+	.divs = {							\
+		RK3328_CLKSEL1(_aclk_core, _pclk_dbg),			\
+	},								\
+}
+
+static struct rockchip_cpuclk_rate_table rk3328_cpuclk_rates[] __initdata = {
+	RK3328_CPUCLK_RATE(1800000000, 1, 7),
+	RK3328_CPUCLK_RATE(1704000000, 1, 7),
+	RK3328_CPUCLK_RATE(1608000000, 1, 7),
+	RK3328_CPUCLK_RATE(1512000000, 1, 7),
+	RK3328_CPUCLK_RATE(1488000000, 1, 5),
+	RK3328_CPUCLK_RATE(1416000000, 1, 5),
+	RK3328_CPUCLK_RATE(1392000000, 1, 5),
+	RK3328_CPUCLK_RATE(1296000000, 1, 5),
+	RK3328_CPUCLK_RATE(1200000000, 1, 5),
+	RK3328_CPUCLK_RATE(1104000000, 1, 5),
+	RK3328_CPUCLK_RATE(1008000000, 1, 5),
+	RK3328_CPUCLK_RATE(912000000, 1, 5),
+	RK3328_CPUCLK_RATE(816000000, 1, 3),
+	RK3328_CPUCLK_RATE(696000000, 1, 3),
+	RK3328_CPUCLK_RATE(600000000, 1, 3),
+	RK3328_CPUCLK_RATE(408000000, 1, 1),
+	RK3328_CPUCLK_RATE(312000000, 1, 1),
+	RK3328_CPUCLK_RATE(216000000,  1, 1),
+	RK3328_CPUCLK_RATE(96000000, 1, 1),
+};
+
+static const struct rockchip_cpuclk_reg_data rk3328_cpuclk_data = {
+	.core_reg = RK3328_CLKSEL_CON(0),
+	.div_core_shift = 0,
+	.div_core_mask = 0x1f,
+	.mux_core_alt = 1,
+	.mux_core_main = 3,
+	.mux_core_shift = 6,
+	.mux_core_mask = 0x3,
+};
+
+PNAME(mux_pll_p)		= { "xin24m", "xin24m" };
+
+PNAME(mux_2plls_p)		= { "cpll", "gpll" };
+PNAME(mux_gpll_cpll_p)		= { "gpll", "cpll" };
+PNAME(mux_cpll_gpll_apll_p)	= { "cpll", "gpll", "apll" };
+PNAME(mux_2plls_xin24m_p)	= { "cpll", "gpll", "xin24m" };
+PNAME(mux_2plls_hdmiphy_p)	= { "cpll", "gpll",
+				    "dummy_hdmiphy" };
+PNAME(mux_4plls_p)		= { "cpll", "gpll",
+				    "dummy_hdmiphy",
+				    "usb480m" };
+PNAME(mux_2plls_u480m_p)	= { "cpll", "gpll",
+				    "usb480m" };
+PNAME(mux_2plls_24m_u480m_p)	= { "cpll", "gpll",
+				     "xin24m", "usb480m" };
+
+PNAME(mux_ddrphy_p)		= { "dpll", "apll", "cpll" };
+PNAME(mux_armclk_p)		= { "apll_core",
+				    "gpll_core",
+				    "dpll_core",
+				    "npll_core"};
+PNAME(mux_hdmiphy_p)		= { "hdmi_phy", "xin24m" };
+PNAME(mux_usb480m_p)		= { "usb480m_phy",
+				    "xin24m" };
+
+PNAME(mux_i2s0_p)		= { "clk_i2s0_div",
+				    "clk_i2s0_frac",
+				    "xin12m",
+				    "xin12m" };
+PNAME(mux_i2s1_p)		= { "clk_i2s1_div",
+				    "clk_i2s1_frac",
+				    "clkin_i2s1",
+				    "xin12m" };
+PNAME(mux_i2s2_p)		= { "clk_i2s2_div",
+				    "clk_i2s2_frac",
+				    "clkin_i2s2",
+				    "xin12m" };
+PNAME(mux_i2s1out_p)		= { "clk_i2s1", "xin12m"};
+PNAME(mux_i2s2out_p)		= { "clk_i2s2", "xin12m" };
+PNAME(mux_spdif_p)		= { "clk_spdif_div",
+				    "clk_spdif_frac",
+				    "xin12m",
+				    "xin12m" };
+PNAME(mux_uart0_p)		= { "clk_uart0_div",
+				    "clk_uart0_frac",
+				    "xin24m" };
+PNAME(mux_uart1_p)		= { "clk_uart1_div",
+				    "clk_uart1_frac",
+				    "xin24m" };
+PNAME(mux_uart2_p)		= { "clk_uart2_div",
+				    "clk_uart2_frac",
+				    "xin24m" };
+
+PNAME(mux_sclk_cif_p)		= { "clk_cif_src",
+				    "xin24m" };
+PNAME(mux_dclk_lcdc_p)		= { "hdmiphy",
+				    "dclk_lcdc_src" };
+PNAME(mux_aclk_peri_pre_p)	= { "cpll_peri",
+				    "gpll_peri",
+				    "hdmiphy_peri" };
+PNAME(mux_ref_usb3otg_src_p)	= { "xin24m",
+				    "clk_usb3otg_ref" };
+PNAME(mux_xin24m_32k_p)		= { "xin24m",
+				    "clk_rtc32k" };
+PNAME(mux_mac2io_src_p)		= { "clk_mac2io_src",
+				    "gmac_clkin" };
+PNAME(mux_mac2phy_src_p)	= { "clk_mac2phy_src",
+				    "phy_50m_out" };
+
+static struct rockchip_pll_clock rk3328_pll_clks[] __initdata = {
+	[apll] = PLL(pll_rk3328, PLL_APLL, "apll", mux_pll_p,
+		     0, RK3328_PLL_CON(0),
+		     RK3328_MODE_CON, 0, 4, 0, rk3328_pll_frac_rates),
+	[dpll] = PLL(pll_rk3328, PLL_DPLL, "dpll", mux_pll_p,
+		     0, RK3328_PLL_CON(8),
+		     RK3328_MODE_CON, 4, 3, 0, NULL),
+	[cpll] = PLL(pll_rk3328, PLL_CPLL, "cpll", mux_pll_p,
+		     0, RK3328_PLL_CON(16),
+		     RK3328_MODE_CON, 8, 2, 0, rk3328_pll_rates),
+	[gpll] = PLL(pll_rk3328, PLL_GPLL, "gpll", mux_pll_p,
+		     0, RK3328_PLL_CON(24),
+		     RK3328_MODE_CON, 12, 1, 0, rk3328_pll_frac_rates),
+	[npll] = PLL(pll_rk3328, PLL_NPLL, "npll", mux_pll_p,
+		     0, RK3328_PLL_CON(40),
+		     RK3328_MODE_CON, 1, 0, 0, rk3328_pll_rates),
+};
+
+#define MFLAGS CLK_MUX_HIWORD_MASK
+#define DFLAGS CLK_DIVIDER_HIWORD_MASK
+#define GFLAGS (CLK_GATE_HIWORD_MASK | CLK_GATE_SET_TO_DISABLE)
+
+static struct rockchip_clk_branch rk3328_i2s0_fracmux __initdata =
+	MUX(0, "i2s0_pre", mux_i2s0_p, CLK_SET_RATE_PARENT,
+	    RK3328_CLKSEL_CON(6), 8, 2, MFLAGS);
+
+static struct rockchip_clk_branch rk3328_i2s1_fracmux __initdata =
+	MUX(0, "i2s1_pre", mux_i2s1_p, CLK_SET_RATE_PARENT,
+	    RK3328_CLKSEL_CON(8), 8, 2, MFLAGS);
+
+static struct rockchip_clk_branch rk3328_i2s2_fracmux __initdata =
+	MUX(0, "i2s2_pre", mux_i2s2_p, CLK_SET_RATE_PARENT,
+	    RK3328_CLKSEL_CON(10), 8, 2, MFLAGS);
+
+static struct rockchip_clk_branch rk3328_spdif_fracmux __initdata =
+	MUX(SCLK_SPDIF, "sclk_spdif", mux_spdif_p, CLK_SET_RATE_PARENT,
+	    RK3328_CLKSEL_CON(12), 8, 2, MFLAGS);
+
+static struct rockchip_clk_branch rk3328_uart0_fracmux __initdata =
+	MUX(SCLK_UART0, "sclk_uart0", mux_uart0_p, CLK_SET_RATE_PARENT,
+	    RK3328_CLKSEL_CON(14), 8, 2, MFLAGS);
+
+static struct rockchip_clk_branch rk3328_uart1_fracmux __initdata =
+	MUX(SCLK_UART1, "sclk_uart1", mux_uart1_p, CLK_SET_RATE_PARENT,
+	    RK3328_CLKSEL_CON(16), 8, 2, MFLAGS);
+
+static struct rockchip_clk_branch rk3328_uart2_fracmux __initdata =
+	MUX(SCLK_UART2, "sclk_uart2", mux_uart2_p, CLK_SET_RATE_PARENT,
+	    RK3328_CLKSEL_CON(18), 8, 2, MFLAGS);
+
+static struct rockchip_clk_branch rk3328_clk_branches[] __initdata = {
+	/*
+	 * Clock-Architecture Diagram 1
+	 */
+
+	DIV(0, "clk_24m", "xin24m", CLK_IGNORE_UNUSED,
+	    RK3328_CLKSEL_CON(2), 8, 5, DFLAGS),
+	COMPOSITE(SCLK_RTC32K, "clk_rtc32k", mux_2plls_xin24m_p, 0,
+		  RK3328_CLKSEL_CON(38), 14, 2, MFLAGS, 0, 14, DFLAGS,
+		  RK3328_CLKGATE_CON(0), 11, GFLAGS),
+	/* PD_MISC */
+	MUX(HDMIPHY, "hdmiphy", mux_hdmiphy_p, CLK_SET_RATE_PARENT,
+	    RK3328_MISC_CON, 13, 1, MFLAGS),
+	MUX(USB480M, "usb480m", mux_usb480m_p, CLK_SET_RATE_PARENT,
+	    RK3328_MISC_CON, 15, 1, MFLAGS),
+
+	/*
+	 * Clock-Architecture Diagram 2
+	 */
+
+	/* PD_CORE */
+	GATE(0, "apll_core", "apll", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(0), 0, GFLAGS),
+	GATE(0, "gpll_core", "gpll", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(0), 2, GFLAGS),
+	GATE(0, "dpll_core", "dpll", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(0), 1, GFLAGS),
+	GATE(0, "npll_core", "npll", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(0), 12, GFLAGS),
+	COMPOSITE_NOMUX(0, "pclk_dbg", "armclk", CLK_IGNORE_UNUSED,
+			RK3328_CLKSEL_CON(1), 0, 4,
+			DFLAGS | CLK_DIVIDER_READ_ONLY,
+			RK3328_CLKGATE_CON(7), 0, GFLAGS),
+	COMPOSITE_NOMUX(0, "aclk_core", "armclk", CLK_IGNORE_UNUSED,
+			RK3328_CLKSEL_CON(1), 4, 3,
+			DFLAGS | CLK_DIVIDER_READ_ONLY,
+			RK3328_CLKGATE_CON(7), 1, GFLAGS),
+	GATE(0, "aclk_core_niu", "aclk_core", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(13), 0, GFLAGS),
+	GATE(0, "aclk_gic400", "aclk_core", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(13), 1, GFLAGS),
+
+	GATE(0, "clk_jtag", "jtag_clkin", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(7), 2, GFLAGS),
+
+	/* PD_GPU */
+	COMPOSITE(0, "aclk_gpu_pre", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(44), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(6), 6, GFLAGS),
+	GATE(ACLK_GPU, "aclk_gpu", "aclk_gpu_pre", CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(14), 0, GFLAGS),
+	GATE(0, "aclk_gpu_niu", "aclk_gpu_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(14), 1, GFLAGS),
+
+	/* PD_DDR */
+	COMPOSITE(0, "clk_ddr", mux_ddrphy_p, CLK_IGNORE_UNUSED,
+		  RK3328_CLKSEL_CON(3), 8, 2, MFLAGS, 0, 3,
+		  DFLAGS | CLK_DIVIDER_POWER_OF_TWO,
+		  RK3328_CLKGATE_CON(0), 4, GFLAGS),
+	GATE(0, "clk_ddrmsch", "clk_ddr", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(18), 6, GFLAGS),
+	GATE(0, "clk_ddrupctl", "clk_ddr", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(18), 5, GFLAGS),
+	GATE(0, "aclk_ddrupctl", "clk_ddr", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(18), 4, GFLAGS),
+	GATE(0, "clk_ddrmon", "xin24m", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(0), 6, GFLAGS),
+
+	COMPOSITE(PCLK_DDR, "pclk_ddr", mux_2plls_hdmiphy_p, 0,
+		  RK3328_CLKSEL_CON(4), 13, 2, MFLAGS, 8, 3, DFLAGS,
+		  RK3328_CLKGATE_CON(7), 4, GFLAGS),
+	GATE(0, "pclk_ddrupctl", "pclk_ddr", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(18), 1, GFLAGS),
+	GATE(0, "pclk_ddr_msch", "pclk_ddr", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(18), 2, GFLAGS),
+	GATE(0, "pclk_ddr_mon", "pclk_ddr", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(18), 3, GFLAGS),
+	GATE(0, "pclk_ddrstdby", "pclk_ddr", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(18), 7, GFLAGS),
+	GATE(0, "pclk_ddr_grf", "pclk_ddr", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(18), 9, GFLAGS),
+
+	/*
+	 * Clock-Architecture Diagram 3
+	 */
+	/* PD_BUS */
+	COMPOSITE(ACLK_BUS_PRE, "aclk_bus_pre", mux_2plls_hdmiphy_p, 0,
+		  RK3328_CLKSEL_CON(0), 13, 2, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(8), 0, GFLAGS),
+	COMPOSITE_NOMUX(HCLK_BUS_PRE, "hclk_bus_pre", "aclk_bus_pre", 0,
+			RK3328_CLKSEL_CON(1), 8, 2, DFLAGS,
+			RK3328_CLKGATE_CON(8), 1, GFLAGS),
+	COMPOSITE_NOMUX(PCLK_BUS_PRE, "pclk_bus_pre", "aclk_bus_pre", 0,
+			RK3328_CLKSEL_CON(1), 12, 3, DFLAGS,
+			RK3328_CLKGATE_CON(8), 2, GFLAGS),
+	GATE(0, "pclk_bus", "pclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(8), 3, GFLAGS),
+	GATE(0, "pclk_phy_pre", "pclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(8), 4, GFLAGS),
+
+	COMPOSITE(SCLK_TSP, "clk_tsp", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(21), 15, 1, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 5, GFLAGS),
+	GATE(0, "clk_hsadc_tsp", "ext_gpio3a2", 0,
+	     RK3328_CLKGATE_CON(17), 13, GFLAGS),
+
+	/* PD_I2S */
+	COMPOSITE(0, "clk_i2s0_div", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(6), 15, 1, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(1), 1, GFLAGS),
+	COMPOSITE_FRACMUX(0, "clk_i2s0_frac", "clk_i2s0_div",
+			  CLK_SET_RATE_PARENT,
+			  RK3328_CLKSEL_CON(7), 0,
+			  RK3328_CLKGATE_CON(1), 2, GFLAGS,
+			  &rk3328_i2s0_fracmux),
+	GATE(SCLK_I2S0, "clk_i2s0", "i2s0_pre", CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(1), 3, GFLAGS),
+
+	COMPOSITE(0, "clk_i2s1_div", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(8), 15, 1, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(1), 4, GFLAGS),
+	COMPOSITE_FRACMUX(0, "clk_i2s1_frac", "clk_i2s1_div",
+			  CLK_SET_RATE_PARENT,
+			  RK3328_CLKSEL_CON(9), 0,
+			  RK3328_CLKGATE_CON(1), 5, GFLAGS,
+			  &rk3328_i2s1_fracmux),
+	GATE(SCLK_I2S1, "clk_i2s1", "i2s1_pre", CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(0), 6, GFLAGS),
+	COMPOSITE_NODIV(SCLK_I2S1_OUT, "i2s1_out", mux_i2s1out_p, 0,
+			RK3328_CLKSEL_CON(8), 12, 1, MFLAGS,
+			RK3328_CLKGATE_CON(1), 7, GFLAGS),
+
+	COMPOSITE(0, "clk_i2s2_div", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(10), 15, 1, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(1), 8, GFLAGS),
+	COMPOSITE_FRACMUX(0, "clk_i2s2_frac", "clk_i2s2_div",
+			  CLK_SET_RATE_PARENT,
+			  RK3328_CLKSEL_CON(11), 0,
+			  RK3328_CLKGATE_CON(1), 9, GFLAGS,
+			  &rk3328_i2s2_fracmux),
+	GATE(SCLK_I2S2, "clk_i2s2", "i2s2_pre", CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(1), 10, GFLAGS),
+	COMPOSITE_NODIV(SCLK_I2S2_OUT, "i2s2_out", mux_i2s2out_p, 0,
+			RK3328_CLKSEL_CON(10), 12, 1, MFLAGS,
+			RK3328_CLKGATE_CON(1), 11, GFLAGS),
+
+	COMPOSITE(0, "clk_spdif_div", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(12), 15, 1, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(1), 12, GFLAGS),
+	COMPOSITE_FRACMUX(0, "clk_spdif_frac", "clk_spdif_div",
+			  CLK_SET_RATE_PARENT,
+			  RK3328_CLKSEL_CON(13), 0,
+			  RK3328_CLKGATE_CON(1), 13, GFLAGS,
+			  &rk3328_spdif_fracmux),
+
+	/* PD_UART */
+	COMPOSITE(0, "clk_uart0_div", mux_2plls_u480m_p, 0,
+		  RK3328_CLKSEL_CON(14), 12, 2, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(1), 14, GFLAGS),
+	COMPOSITE(0, "clk_uart1_div", mux_2plls_u480m_p, 0,
+		  RK3328_CLKSEL_CON(16), 12, 2, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 0, GFLAGS),
+	COMPOSITE(0, "clk_uart2_div", mux_2plls_u480m_p,
+		  0, RK3328_CLKSEL_CON(18), 12, 2, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 2, GFLAGS),
+	COMPOSITE_FRACMUX(0, "clk_uart0_frac", "clk_uart0_div",
+			  CLK_SET_RATE_PARENT,
+			  RK3328_CLKSEL_CON(15), 0,
+			  RK3328_CLKGATE_CON(1), 15, GFLAGS,
+			  &rk3328_uart0_fracmux),
+	COMPOSITE_FRACMUX(0, "clk_uart1_frac", "clk_uart1_div",
+			  CLK_SET_RATE_PARENT,
+			  RK3328_CLKSEL_CON(17), 0,
+			  RK3328_CLKGATE_CON(2), 1, GFLAGS,
+			  &rk3328_uart1_fracmux),
+	COMPOSITE_FRACMUX(0, "clk_uart2_frac", "clk_uart2_div",
+			  CLK_SET_RATE_PARENT,
+			  RK3328_CLKSEL_CON(19), 0,
+			  RK3328_CLKGATE_CON(2), 3, GFLAGS,
+			  &rk3328_uart2_fracmux),
+
+	/*
+	 * Clock-Architecture Diagram 4
+	 */
+	 COMPOSITE(SCLK_I2C0, "clk_i2c0", mux_2plls_p, 0,
+		   RK3328_CLKSEL_CON(34), 7, 1, MFLAGS, 0, 7, DFLAGS,
+		   RK3328_CLKGATE_CON(2), 9, GFLAGS),
+	COMPOSITE(SCLK_I2C1, "clk_i2c1", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(34), 15, 1, MFLAGS, 8, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 10, GFLAGS),
+	COMPOSITE(SCLK_I2C2, "clk_i2c2", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(35), 7, 1, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 11, GFLAGS),
+	COMPOSITE(SCLK_I2C3, "clk_i2c3", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(35), 15, 1, MFLAGS, 8, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 12, GFLAGS),
+	COMPOSITE(SCLK_CRYPTO, "clk_crypto", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(20), 7, 1, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 4, GFLAGS),
+	COMPOSITE_NOMUX(SCLK_TSADC, "clk_tsadc", "clk_24m", 0,
+			RK3328_CLKSEL_CON(22), 0, 10, DFLAGS,
+			RK3328_CLKGATE_CON(2), 6, GFLAGS),
+	COMPOSITE_NOMUX(SCLK_SARADC, "clk_saradc", "clk_24m", 0,
+			RK3328_CLKSEL_CON(23), 0, 10, DFLAGS,
+			RK3328_CLKGATE_CON(2), 14, GFLAGS),
+	COMPOSITE(SCLK_SPI, "clk_spi", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(24), 7, 1, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 7, GFLAGS),
+	COMPOSITE(SCLK_PWM, "clk_pwm", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(24), 15, 1, MFLAGS, 8, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 8, GFLAGS),
+	COMPOSITE(SCLK_OTP, "clk_otp", mux_2plls_xin24m_p, 0,
+		  RK3328_CLKSEL_CON(4), 6, 2, MFLAGS, 0, 6, DFLAGS,
+		  RK3328_CLKGATE_CON(3), 8, GFLAGS),
+	COMPOSITE(SCLK_EFUSE, "clk_efuse", mux_2plls_xin24m_p, 0,
+		  RK3328_CLKSEL_CON(5), 14, 2, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 13, GFLAGS),
+	COMPOSITE(SCLK_PDM, "clk_pdm", mux_cpll_gpll_apll_p,
+		  CLK_SET_RATE_NO_REPARENT | CLK_SET_RATE_PARENT,
+		  RK3328_CLKSEL_CON(20), 14, 2, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(2), 15, GFLAGS),
+
+	GATE(SCLK_TIMER0, "sclk_timer0", "xin24m", 0,
+	     RK3328_CLKGATE_CON(8), 5, GFLAGS),
+	GATE(SCLK_TIMER1, "sclk_timer1", "xin24m", 0,
+	     RK3328_CLKGATE_CON(8), 6, GFLAGS),
+	GATE(SCLK_TIMER2, "sclk_timer2", "xin24m", 0,
+	     RK3328_CLKGATE_CON(8), 7, GFLAGS),
+	GATE(SCLK_TIMER3, "sclk_timer3", "xin24m", 0,
+	     RK3328_CLKGATE_CON(8), 8, GFLAGS),
+	GATE(SCLK_TIMER4, "sclk_timer4", "xin24m", 0,
+	     RK3328_CLKGATE_CON(8), 9, GFLAGS),
+	GATE(SCLK_TIMER5, "sclk_timer5", "xin24m", 0,
+	     RK3328_CLKGATE_CON(8), 10, GFLAGS),
+
+	COMPOSITE(SCLK_WIFI, "clk_wifi", mux_2plls_u480m_p, 0,
+		  RK3328_CLKSEL_CON(52), 6, 2, MFLAGS, 0, 6, DFLAGS,
+		  RK3328_CLKGATE_CON(0), 10, GFLAGS),
+
+	/*
+	 * Clock-Architecture Diagram 5
+	 */
+	/* PD_VIDEO */
+	COMPOSITE(ACLK_RKVDEC_PRE, "aclk_rkvdec_pre", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(48), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(6), 0, GFLAGS),
+	FACTOR_GATE(HCLK_RKVDEC_PRE, "hclk_rkvdec_pre", "aclk_rkvdec_pre",
+		    0, 1, 4,
+		    RK3328_CLKGATE_CON(11), 0, GFLAGS),
+	GATE(ACLK_RKVDEC, "aclk_rkvdec", "aclk_rkvdec_pre", CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(24), 0, GFLAGS),
+	GATE(HCLK_RKVDEC, "hclk_rkvdec", "hclk_rkvdec_pre", CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(24), 1, GFLAGS),
+	GATE(0, "aclk_rkvdec_niu", "aclk_rkvdec_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(24), 2, GFLAGS),
+	GATE(0, "hclk_rkvdec_niu", "hclk_rkvdec_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(24), 3, GFLAGS),
+
+	COMPOSITE(SCLK_VDEC_CABAC, "sclk_vdec_cabac", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(48), 14, 2, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(6), 1, GFLAGS),
+
+	COMPOSITE(SCLK_VDEC_CORE, "sclk_vdec_core", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(49), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(6), 2, GFLAGS),
+
+	COMPOSITE(ACLK_VPU_PRE, "aclk_vpu_pre", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(50), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(6), 5, GFLAGS),
+	FACTOR_GATE(HCLK_VPU_PRE, "hclk_vpu_pre", "aclk_vpu_pre", 0, 1, 4,
+		    RK3328_CLKGATE_CON(11), 8, GFLAGS),
+	GATE(ACLK_VPU, "aclk_vpu", "aclk_vpu_pre", CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(23), 0, GFLAGS),
+	GATE(HCLK_VPU, "hclk_vpu", "hclk_vpu_pre", CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(23), 1, GFLAGS),
+	GATE(0, "aclk_vpu_niu", "aclk_vpu_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(23), 2, GFLAGS),
+	GATE(0, "hclk_vpu_niu", "hclk_vpu_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(23), 3, GFLAGS),
+
+	COMPOSITE(ACLK_RKVENC, "aclk_rkvenc", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(51), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(6), 3, GFLAGS),
+	FACTOR_GATE(HCLK_RKVENC, "hclk_rkvenc", "aclk_rkvenc", 0, 1, 4,
+		    RK3328_CLKGATE_CON(11), 4, GFLAGS),
+	GATE(0, "aclk_rkvenc_niu", "aclk_rkvenc", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(25), 0, GFLAGS),
+	GATE(0, "hclk_rkvenc_niu", "hclk_rkvenc", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(25), 1, GFLAGS),
+	GATE(ACLK_H265, "aclk_h265", "aclk_rkvenc", 0,
+	     RK3328_CLKGATE_CON(25), 0, GFLAGS),
+	GATE(PCLK_H265, "pclk_h265", "hclk_rkvenc", 0,
+	     RK3328_CLKGATE_CON(25), 1, GFLAGS),
+	GATE(ACLK_H264, "aclk_h264", "aclk_rkvenc", 0,
+	     RK3328_CLKGATE_CON(25), 0, GFLAGS),
+	GATE(HCLK_H264, "hclk_h264", "hclk_rkvenc", 0,
+	     RK3328_CLKGATE_CON(25), 1, GFLAGS),
+	GATE(ACLK_AXISRAM, "aclk_axisram", "aclk_rkvenc", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(25), 0, GFLAGS),
+
+	COMPOSITE(SCLK_VENC_CORE, "sclk_venc_core", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(51), 14, 2, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(6), 4, GFLAGS),
+
+	COMPOSITE(SCLK_VENC_DSP, "sclk_venc_dsp", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(52), 14, 2, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(6), 7, GFLAGS),
+
+	/*
+	 * Clock-Architecture Diagram 6
+	 */
+	/* PD_VIO */
+	COMPOSITE(ACLK_VIO_PRE, "aclk_vio_pre", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(37), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(5), 2, GFLAGS),
+	DIV(HCLK_VIO_PRE, "hclk_vio_pre", "aclk_vio_pre", 0,
+	    RK3328_CLKSEL_CON(37), 8, 5, DFLAGS),
+
+	COMPOSITE(ACLK_RGA_PRE, "aclk_rga_pre", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(36), 14, 2, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(5), 0, GFLAGS),
+	COMPOSITE(SCLK_RGA, "clk_rga", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(36), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(5), 1, GFLAGS),
+	COMPOSITE(ACLK_VOP_PRE, "aclk_vop_pre", mux_4plls_p, 0,
+		  RK3328_CLKSEL_CON(39), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(5), 5, GFLAGS),
+	GATE(0, "clk_hdmi_sfc", "xin24m", 0,
+	     RK3328_CLKGATE_CON(5), 4, GFLAGS),
+
+	COMPOSITE_NODIV(0, "clk_cif_src", mux_2plls_p, 0,
+			RK3328_CLKSEL_CON(42), 7, 1, MFLAGS,
+			RK3328_CLKGATE_CON(5), 3, GFLAGS),
+	COMPOSITE_NOGATE(SCLK_CIF_OUT, "clk_cif_out", mux_sclk_cif_p,
+			 CLK_SET_RATE_PARENT,
+			 RK3328_CLKSEL_CON(42), 5, 1, MFLAGS, 0, 5, DFLAGS),
+
+	COMPOSITE(DCLK_LCDC_SRC, "dclk_lcdc_src", mux_gpll_cpll_p, 0,
+		  RK3328_CLKSEL_CON(40), 0, 1, MFLAGS, 8, 8, DFLAGS,
+		  RK3328_CLKGATE_CON(5), 6, GFLAGS),
+	DIV(DCLK_HDMIPHY, "dclk_hdmiphy", "dclk_lcdc_src", 0,
+	    RK3328_CLKSEL_CON(40), 3, 3, DFLAGS),
+	MUX(DCLK_LCDC, "dclk_lcdc", mux_dclk_lcdc_p, 0,
+	    RK3328_CLKSEL_CON(40), 1, 1, MFLAGS),
+
+	/*
+	 * Clock-Architecture Diagram 7
+	 */
+	/* PD_PERI */
+	GATE(0, "gpll_peri", "gpll", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(4), 0, GFLAGS),
+	GATE(0, "cpll_peri", "cpll", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(4), 1, GFLAGS),
+	GATE(0, "hdmiphy_peri", "hdmiphy", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(4), 2, GFLAGS),
+	COMPOSITE_NOGATE(ACLK_PERI_PRE, "aclk_peri_pre", mux_aclk_peri_pre_p, 0,
+			 RK3328_CLKSEL_CON(28), 6, 2, MFLAGS, 0, 5, DFLAGS),
+	COMPOSITE_NOMUX(PCLK_PERI, "pclk_peri", "aclk_peri_pre",
+			CLK_IGNORE_UNUSED,
+			RK3328_CLKSEL_CON(29), 0, 2, DFLAGS,
+			RK3328_CLKGATE_CON(10), 2, GFLAGS),
+	COMPOSITE_NOMUX(HCLK_PERI, "hclk_peri", "aclk_peri_pre",
+			CLK_IGNORE_UNUSED,
+			RK3328_CLKSEL_CON(29), 4, 3, DFLAGS,
+			RK3328_CLKGATE_CON(10), 1, GFLAGS),
+	GATE(ACLK_PERI, "aclk_peri", "aclk_peri_pre",
+	     CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT,
+	     RK3328_CLKGATE_CON(10), 0, GFLAGS),
+
+	COMPOSITE(SCLK_SDMMC, "clk_sdmmc", mux_2plls_24m_u480m_p, 0,
+		  RK3328_CLKSEL_CON(30), 8, 2, MFLAGS, 0, 8, DFLAGS,
+		  RK3328_CLKGATE_CON(4), 3, GFLAGS),
+
+	COMPOSITE(SCLK_SDIO, "clk_sdio", mux_2plls_24m_u480m_p, 0,
+		  RK3328_CLKSEL_CON(31), 8, 2, MFLAGS, 0, 8, DFLAGS,
+		  RK3328_CLKGATE_CON(4), 4, GFLAGS),
+
+	COMPOSITE(SCLK_EMMC, "clk_emmc", mux_2plls_24m_u480m_p, 0,
+		  RK3328_CLKSEL_CON(32), 8, 2, MFLAGS, 0, 8, DFLAGS,
+		  RK3328_CLKGATE_CON(4), 5, GFLAGS),
+
+	COMPOSITE(SCLK_SDMMC_EXT, "clk_sdmmc_ext",
+		  mux_2plls_24m_u480m_p, 0,
+		  RK3328_CLKSEL_CON(43), 8, 2, MFLAGS, 0, 8, DFLAGS,
+		  RK3328_CLKGATE_CON(4), 10, GFLAGS),
+
+	COMPOSITE(SCLK_REF_USB3OTG_SRC, "clk_ref_usb3otg_src", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(45), 7, 1, MFLAGS, 0, 7, DFLAGS,
+		  RK3328_CLKGATE_CON(4), 9, GFLAGS),
+
+	MUX(SCLK_REF_USB3OTG, "clk_ref_usb3otg",
+	    mux_ref_usb3otg_src_p, CLK_SET_RATE_PARENT,
+	    RK3328_CLKSEL_CON(45), 8, 1, MFLAGS),
+
+	GATE(SCLK_USB3OTG_REF, "clk_usb3otg_ref", "xin24m", 0,
+	     RK3328_CLKGATE_CON(4), 7, GFLAGS),
+
+	COMPOSITE(SCLK_USB3OTG_SUSPEND, "clk_usb3otg_suspend",
+		  mux_xin24m_32k_p, 0,
+		  RK3328_CLKSEL_CON(33), 15, 1, MFLAGS, 0, 10, DFLAGS,
+		  RK3328_CLKGATE_CON(4), 8, GFLAGS),
+
+	/*
+	 * Clock-Architecture Diagram 8
+	 */
+	/* PD_GMAC */
+
+	COMPOSITE(ACLK_GMAC, "aclk_gmac", mux_2plls_hdmiphy_p, 0,
+		  RK3328_CLKSEL_CON(35), 6, 2, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(3), 2, GFLAGS),
+	COMPOSITE_NOMUX(PCLK_GMAC, "pclk_gmac", "aclk_gmac", 0,
+			RK3328_CLKSEL_CON(25), 8, 3, DFLAGS,
+			RK3328_CLKGATE_CON(9), 0, GFLAGS),
+
+	COMPOSITE(SCLK_MAC2IO_SRC, "clk_mac2io_src", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(27), 7, 1, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(3), 1, GFLAGS),
+	GATE(SCLK_MAC2IO_REF, "clk_mac2io_ref", "clk_mac2io", 0,
+	     RK3328_CLKGATE_CON(9), 7, GFLAGS),
+	GATE(SCLK_MAC2IO_RX, "clk_mac2io_rx", "clk_mac2io", 0,
+	     RK3328_CLKGATE_CON(9), 4, GFLAGS),
+	GATE(SCLK_MAC2IO_TX, "clk_mac2io_tx", "clk_mac2io", 0,
+	     RK3328_CLKGATE_CON(9), 5, GFLAGS),
+	GATE(SCLK_MAC2IO_REFOUT, "clk_mac2io_refout", "clk_mac2io", 0,
+	     RK3328_CLKGATE_CON(9), 6, GFLAGS),
+	COMPOSITE(SCLK_MAC2IO_OUT, "clk_mac2io_out", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(27), 15, 1, MFLAGS, 8, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(3), 5, GFLAGS),
+
+	COMPOSITE(SCLK_MAC2PHY_SRC, "clk_mac2phy_src", mux_2plls_p, 0,
+		  RK3328_CLKSEL_CON(26), 7, 1, MFLAGS, 0, 5, DFLAGS,
+		  RK3328_CLKGATE_CON(3), 0, GFLAGS),
+	GATE(SCLK_MAC2PHY_REF, "clk_mac2phy_ref", "clk_mac2phy", 0,
+	     RK3328_CLKGATE_CON(9), 3, GFLAGS),
+	GATE(SCLK_MAC2PHY_RXTX, "clk_mac2phy_rxtx", "clk_mac2phy", 0,
+	     RK3328_CLKGATE_CON(9), 1, GFLAGS),
+	COMPOSITE_NOMUX(SCLK_MAC2PHY_OUT, "clk_mac2phy_out", "clk_mac2phy", 0,
+			RK3328_CLKSEL_CON(26), 8, 2, DFLAGS,
+			RK3328_CLKGATE_CON(9), 2, GFLAGS),
+
+	FACTOR(0, "xin12m", "xin24m", 0, 1, 2),
+
+	/*
+	 * Clock-Architecture Diagram 9
+	 */
+
+	/* PD_VOP */
+	GATE(ACLK_RGA, "aclk_rga", "aclk_rga_pre", 0,
+	     RK3328_CLKGATE_CON(21), 10, GFLAGS),
+	GATE(0, "aclk_rga_niu", "aclk_rga_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(22), 3, GFLAGS),
+	GATE(ACLK_VOP, "aclk_vop", "aclk_vop_pre", 0,
+	     RK3328_CLKGATE_CON(21), 2, GFLAGS),
+	GATE(0, "aclk_vop_niu", "aclk_vop_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(21), 4, GFLAGS),
+
+	GATE(ACLK_IEP, "aclk_iep", "aclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(21), 6, GFLAGS),
+	GATE(ACLK_CIF, "aclk_cif", "aclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(21), 8, GFLAGS),
+	GATE(ACLK_HDCP, "aclk_hdcp", "aclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(21), 15, GFLAGS),
+	GATE(0, "aclk_vio_niu", "aclk_vio_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(22), 2, GFLAGS),
+
+	GATE(HCLK_VOP, "hclk_vop", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(21), 3, GFLAGS),
+	GATE(0, "hclk_vop_niu", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(21), 5, GFLAGS),
+	GATE(HCLK_IEP, "hclk_iep", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(21), 7, GFLAGS),
+	GATE(HCLK_CIF, "hclk_cif", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(21), 9, GFLAGS),
+	GATE(HCLK_RGA, "hclk_rga", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(21), 11, GFLAGS),
+	GATE(0, "hclk_ahb1tom", "hclk_vio_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(21), 12, GFLAGS),
+	GATE(0, "pclk_vio_h2p", "hclk_vio_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(21), 13, GFLAGS),
+	GATE(0, "hclk_vio_h2p", "hclk_vio_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(21), 14, GFLAGS),
+	GATE(HCLK_HDCP, "hclk_hdcp", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(22), 0, GFLAGS),
+	GATE(HCLK_VIO, "hclk_vio", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(22), 1, GFLAGS),
+	GATE(PCLK_HDMI, "pclk_hdmi", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(22), 4, GFLAGS),
+	GATE(PCLK_HDCP, "pclk_hdcp", "hclk_vio_pre", 0,
+	     RK3328_CLKGATE_CON(22), 5, GFLAGS),
+
+	/* PD_PERI */
+	GATE(0, "aclk_peri_noc", "aclk_peri", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(19), 11, GFLAGS),
+	GATE(ACLK_USB3OTG, "aclk_usb3otg", "aclk_peri", 0,
+	     RK3328_CLKGATE_CON(19), 4, GFLAGS),
+
+	GATE(HCLK_SDMMC, "hclk_sdmmc", "hclk_peri", 0,
+	     RK3328_CLKGATE_CON(19), 0, GFLAGS),
+	GATE(HCLK_SDIO, "hclk_sdio", "hclk_peri", 0,
+	     RK3328_CLKGATE_CON(19), 1, GFLAGS),
+	GATE(HCLK_EMMC, "hclk_emmc", "hclk_peri", 0,
+	     RK3328_CLKGATE_CON(19), 2, GFLAGS),
+	GATE(HCLK_SDMMC_EXT, "hclk_sdmmc_ext", "hclk_peri", 0,
+	     RK3328_CLKGATE_CON(19), 15, GFLAGS),
+	GATE(HCLK_HOST0, "hclk_host0", "hclk_peri", 0,
+	     RK3328_CLKGATE_CON(19), 6, GFLAGS),
+	GATE(HCLK_HOST0_ARB, "hclk_host0_arb", "hclk_peri", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(19), 7, GFLAGS),
+	GATE(HCLK_OTG, "hclk_otg", "hclk_peri", 0,
+	     RK3328_CLKGATE_CON(19), 8, GFLAGS),
+	GATE(HCLK_OTG_PMU, "hclk_otg_pmu", "hclk_peri", 0,
+	     RK3328_CLKGATE_CON(19), 9, GFLAGS),
+	GATE(0, "hclk_peri_niu", "hclk_peri", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(19), 12, GFLAGS),
+	GATE(0, "pclk_peri_niu", "hclk_peri", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(19), 13, GFLAGS),
+
+	/* PD_GMAC */
+	GATE(ACLK_MAC2PHY, "aclk_mac2phy", "aclk_gmac", 0,
+	     RK3328_CLKGATE_CON(26), 0, GFLAGS),
+	GATE(ACLK_MAC2IO, "aclk_mac2io", "aclk_gmac", 0,
+	     RK3328_CLKGATE_CON(26), 2, GFLAGS),
+	GATE(0, "aclk_gmac_niu", "aclk_gmac", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(26), 4, GFLAGS),
+	GATE(PCLK_MAC2PHY, "pclk_mac2phy", "pclk_gmac", 0,
+	     RK3328_CLKGATE_CON(26), 1, GFLAGS),
+	GATE(PCLK_MAC2IO, "pclk_mac2io", "pclk_gmac", 0,
+	     RK3328_CLKGATE_CON(26), 3, GFLAGS),
+	GATE(0, "pclk_gmac_niu", "pclk_gmac", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(26), 5, GFLAGS),
+
+	/* PD_BUS */
+	GATE(0, "aclk_bus_niu", "aclk_bus_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(15), 12, GFLAGS),
+	GATE(ACLK_DCF, "aclk_dcf", "aclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(15), 11, GFLAGS),
+	GATE(ACLK_TSP, "aclk_tsp", "aclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(17), 12, GFLAGS),
+	GATE(0, "aclk_intmem", "aclk_bus_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(15), 0, GFLAGS),
+	GATE(ACLK_DMAC, "aclk_dmac_bus", "aclk_bus_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(15), 1, GFLAGS),
+
+	GATE(0, "hclk_rom", "hclk_bus_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(15), 2, GFLAGS),
+	GATE(HCLK_I2S0_8CH, "hclk_i2s0_8ch", "hclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(15), 3, GFLAGS),
+	GATE(HCLK_I2S1_8CH, "hclk_i2s1_8ch", "hclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(15), 4, GFLAGS),
+	GATE(HCLK_I2S2_2CH, "hclk_i2s2_2ch", "hclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(15), 5, GFLAGS),
+	GATE(HCLK_SPDIF_8CH, "hclk_spdif_8ch", "hclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(15), 6, GFLAGS),
+	GATE(HCLK_TSP, "hclk_tsp", "hclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(17), 11, GFLAGS),
+	GATE(HCLK_CRYPTO_MST, "hclk_crypto_mst", "hclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(15), 7, GFLAGS),
+	GATE(HCLK_CRYPTO_SLV, "hclk_crypto_slv", "hclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(15), 8, GFLAGS),
+	GATE(0, "hclk_bus_niu", "hclk_bus_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(15), 13, GFLAGS),
+	GATE(HCLK_PDM, "hclk_pdm", "hclk_bus_pre", 0,
+	     RK3328_CLKGATE_CON(28), 0, GFLAGS),
+
+	GATE(0, "pclk_bus_niu", "pclk_bus", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(15), 14, GFLAGS),
+	GATE(0, "pclk_efuse", "pclk_bus", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(15), 9, GFLAGS),
+	GATE(0, "pclk_otp", "pclk_bus", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(28), 4, GFLAGS),
+	GATE(PCLK_I2C0, "pclk_i2c0", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(15), 10, GFLAGS),
+	GATE(PCLK_I2C1, "pclk_i2c1", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 0, GFLAGS),
+	GATE(PCLK_I2C2, "pclk_i2c2", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 1, GFLAGS),
+	GATE(PCLK_I2C3, "pclk_i2c3", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 2, GFLAGS),
+	GATE(PCLK_TIMER, "pclk_timer0", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 3, GFLAGS),
+	GATE(0, "pclk_stimer", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 4, GFLAGS),
+	GATE(PCLK_SPI, "pclk_spi", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 5, GFLAGS),
+	GATE(PCLK_PWM, "pclk_rk_pwm", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 6, GFLAGS),
+	GATE(PCLK_GPIO0, "pclk_gpio0", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 7, GFLAGS),
+	GATE(PCLK_GPIO1, "pclk_gpio1", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 8, GFLAGS),
+	GATE(PCLK_GPIO2, "pclk_gpio2", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 9, GFLAGS),
+	GATE(PCLK_GPIO3, "pclk_gpio3", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 10, GFLAGS),
+	GATE(PCLK_UART0, "pclk_uart0", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 11, GFLAGS),
+	GATE(PCLK_UART1, "pclk_uart1", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 12, GFLAGS),
+	GATE(PCLK_UART2, "pclk_uart2", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 13, GFLAGS),
+	GATE(PCLK_TSADC, "pclk_tsadc", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 14, GFLAGS),
+	GATE(PCLK_DCF, "pclk_dcf", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(16), 15, GFLAGS),
+	GATE(PCLK_GRF, "pclk_grf", "pclk_bus", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 0, GFLAGS),
+	GATE(0, "pclk_cru", "pclk_bus", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 4, GFLAGS),
+	GATE(0, "pclk_sgrf", "pclk_bus", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 6, GFLAGS),
+	GATE(0, "pclk_sim", "pclk_bus", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 10, GFLAGS),
+	GATE(PCLK_SARADC, "pclk_saradc", "pclk_bus", 0,
+	     RK3328_CLKGATE_CON(17), 15, GFLAGS),
+	GATE(0, "pclk_pmu", "pclk_bus", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(28), 3, GFLAGS),
+
+	GATE(PCLK_USB3PHY_OTG, "pclk_usb3phy_otg", "pclk_phy_pre", 0,
+	     RK3328_CLKGATE_CON(28), 1, GFLAGS),
+	GATE(PCLK_USB3PHY_PIPE, "pclk_usb3phy_pipe", "pclk_phy_pre", 0,
+	     RK3328_CLKGATE_CON(28), 2, GFLAGS),
+	GATE(PCLK_USB3_GRF, "pclk_usb3_grf", "pclk_phy_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 2, GFLAGS),
+	GATE(PCLK_USB2_GRF, "pclk_usb2_grf", "pclk_phy_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 14, GFLAGS),
+	GATE(0, "pclk_ddrphy", "pclk_phy_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 13, GFLAGS),
+	GATE(0, "pclk_acodecphy", "pclk_phy_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 5, GFLAGS),
+	GATE(PCLK_HDMIPHY, "pclk_hdmiphy", "pclk_phy_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 7, GFLAGS),
+	GATE(0, "pclk_vdacphy", "pclk_phy_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(17), 8, GFLAGS),
+	GATE(0, "pclk_phy_niu", "pclk_phy_pre", CLK_IGNORE_UNUSED,
+	     RK3328_CLKGATE_CON(15), 15, GFLAGS),
+
+	/* PD_MMC */
+	MMC(SCLK_SDMMC_DRV, "sdmmc_drv", "sclk_sdmmc",
+	    RK3328_SDMMC_CON0, 1),
+	MMC(SCLK_SDMMC_SAMPLE, "sdmmc_sample", "sclk_sdmmc",
+	    RK3328_SDMMC_CON1, 0),
+
+	MMC(SCLK_SDIO_DRV, "sdio_drv", "sclk_sdio",
+	    RK3328_SDIO_CON0, 1),
+	MMC(SCLK_SDIO_SAMPLE, "sdio_sample", "sclk_sdio",
+	    RK3328_SDIO_CON1, 0),
+
+	MMC(SCLK_EMMC_DRV, "emmc_drv", "sclk_emmc",
+	    RK3328_EMMC_CON0, 1),
+	MMC(SCLK_EMMC_SAMPLE, "emmc_sample", "sclk_emmc",
+	    RK3328_EMMC_CON1, 0),
+
+	MMC(SCLK_SDMMC_EXT_DRV, "sdmmc_ext_drv", "sclk_sdmmc_ext",
+	    RK3328_SDMMC_EXT_CON0, 1),
+	MMC(SCLK_SDMMC_EXT_SAMPLE, "sdmmc_ext_sample", "sclk_sdmmc_ext",
+	    RK3328_SDMMC_EXT_CON1, 0),
+};
+
+static struct rockchip_clk_branch rk3328_clk_grf_branches[] __initdata = {
+	/*
+	 * GRF CRU Clock-Architecture
+	 */
+	MUX(SCLK_MAC2IO, "clk_mac2io", mux_mac2io_src_p, 0,
+	    RK3328_GRF_MAC_CON1, 10, 1, MFLAGS),
+	MUX(SCLK_MAC2PHY, "clk_mac2phy", mux_mac2phy_src_p, 0,
+	    RK3328_GRF_MAC_CON2, 10, 1, MFLAGS),
+};
+
+static const char *const rk3328_critical_clocks[] __initconst = {
+	"aclk_bus",
+	"pclk_bus",
+	"hclk_bus",
+	"aclk_peri",
+	"hclk_peri",
+	"pclk_peri",
+	"pclk_dbg",
+	"aclk_core_niu",
+	"aclk_gic400",
+	"aclk_intmem",
+	"hclk_rom",
+	"pclk_grf",
+	"pclk_cru",
+	"pclk_sgrf",
+	"pclk_timer0",
+	"clk_timer0",
+	"pclk_ddr_msch",
+	"pclk_ddr_mon",
+	"pclk_ddr_grf",
+	"clk_ddrupctl",
+	"clk_ddrmsch",
+	"hclk_ahb1tom",
+	"clk_jtag",
+	"pclk_ddrphy",
+	"pclk_pmu",
+	"hclk_otg_pmu",
+	"aclk_rga_niu",
+	"pclk_vio_h2p",
+	"hclk_vio_h2p",
+};
+
+static void __iomem *rk3328_cru_base;
+
+void rk3328_dump_cru(void)
+{
+	if (rk3328_cru_base) {
+		pr_warn("CRU:\n");
+		print_hex_dump(KERN_WARNING, "", DUMP_PREFIX_OFFSET,
+			       32, 4, rk3328_cru_base,
+			       0x400, false);
+	}
+}
+EXPORT_SYMBOL_GPL(rk3328_dump_cru);
+
+static int rk3328_clk_panic(struct notifier_block *this,
+			    unsigned long ev, void *ptr)
+{
+	rk3328_dump_cru();
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block rk3328_clk_panic_block = {
+	.notifier_call = rk3328_clk_panic,
+};
+
+static void __init rk3328_clk_init(struct device_node *np)
+{
+	struct rockchip_clk_provider *ctx;
+	void __iomem *reg_base;
+
+	reg_base = of_iomap(np, 0);
+	if (!reg_base) {
+		pr_err("%s: could not map cru region\n", __func__);
+		return;
+	}
+
+	rk3328_cru_base = reg_base;
+
+	ctx = rockchip_clk_init(np, reg_base, CLK_NR_CLKS);
+	if (IS_ERR(ctx)) {
+		pr_err("%s: rockchip clk init failed\n", __func__);
+		iounmap(reg_base);
+		return;
+	}
+
+	rockchip_clk_register_plls(ctx, rk3328_pll_clks,
+				   ARRAY_SIZE(rk3328_pll_clks),
+				   RK3328_GRF_SOC_STATUS0);
+	rockchip_clk_register_branches(ctx, rk3328_clk_branches,
+				       ARRAY_SIZE(rk3328_clk_branches));
+	rockchip_clk_protect_critical(rk3328_critical_clocks,
+				      ARRAY_SIZE(rk3328_critical_clocks));
+
+	rockchip_clk_register_armclk(ctx, ARMCLK, "armclk",
+				     mux_armclk_p, ARRAY_SIZE(mux_armclk_p),
+				     &rk3328_cpuclk_data, rk3328_cpuclk_rates,
+				     ARRAY_SIZE(rk3328_cpuclk_rates));
+
+	rockchip_register_softrst(np, 11, reg_base + RK3328_SOFTRST_CON(0),
+				  ROCKCHIP_SOFTRST_HIWORD_MASK);
+
+	rockchip_register_restart_notifier(ctx, RK3328_GLB_SRST_FST, NULL);
+
+	rockchip_clk_of_add_provider(np, ctx);
+
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &rk3328_clk_panic_block);
+}
+
+CLK_OF_DECLARE(rk3328_cru, "rockchip,rk3328-cru", rk3328_clk_init);
+
+static void __init rk3328_grf_clk_init(struct device_node *np)
+{
+	struct rockchip_clk_provider *ctx;
+	void __iomem *reg_base;
+
+	reg_base = of_iomap(np, 0);
+	if (!reg_base) {
+		pr_err("%s: could not map cru pmu region\n", __func__);
+		return;
+	}
+
+	ctx = rockchip_clk_init(np, reg_base, CLKGRF_NR_CLKS);
+	if (IS_ERR(ctx)) {
+		pr_err("%s: rockchip pmu clk init failed\n", __func__);
+		return;
+	}
+
+	rockchip_clk_register_branches(ctx, rk3328_clk_grf_branches,
+				       ARRAY_SIZE(rk3328_clk_grf_branches));
+
+	rockchip_clk_of_add_provider(np, ctx);
+}
+
+CLK_OF_DECLARE(rk3328_cru_grf, "rockchip,rk3328-grf", rk3328_grf_clk_init);
diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h
index d67eecc4ade9..7225997f8d52 100644
--- a/drivers/clk/rockchip/clk.h
+++ b/drivers/clk/rockchip/clk.h
@@ -91,6 +91,28 @@
 #define RK3288_EMMC_CON0		0x218
 #define RK3288_EMMC_CON1		0x21c
 
+#define RK3328_PLL_CON(x)		RK2928_PLL_CON(x)
+#define RK3328_CLKSEL_CON(x)		((x) * 0x4 + 0x100)
+#define RK3328_CLKGATE_CON(x)		((x) * 0x4 + 0x200)
+#define RK3328_GRFCLKSEL_CON(x)		((x) * 0x4 + 0x100)
+#define RK3328_GLB_SRST_FST		0x9c
+#define RK3328_GLB_SRST_SND		0x98
+#define RK3328_SOFTRST_CON(x)		((x) * 0x4 + 0x300)
+#define RK3328_MODE_CON			0x80
+#define RK3328_MISC_CON			0x84
+#define RK3328_DIV_ACLKM_MASK		0x7
+#define RK3328_DIV_ACLKM_SHIFT		4
+#define RK3328_DIV_PCLK_DBG_MASK	0xf
+#define RK3328_DIV_PCLK_DBG_SHIFT	0
+#define RK3328_SDMMC_CON0		0x380
+#define RK3328_SDMMC_CON1		0x384
+#define RK3328_SDIO_CON0		0x388
+#define RK3328_SDIO_CON1		0x38c
+#define RK3328_EMMC_CON0		0x390
+#define RK3328_EMMC_CON1		0x394
+#define RK3328_SDMMC_EXT_CON0		0x398
+#define RK3328_SDMMC_EXT_CON1		0x39C
+
 #define RK3368_PLL_CON(x)		RK2928_PLL_CON(x)
 #define RK3368_CLKSEL_CON(x)		((x) * 0x4 + 0x100)
 #define RK3368_CLKGATE_CON(x)		((x) * 0x4 + 0x200)
@@ -130,6 +152,7 @@
 enum rockchip_pll_type {
 	pll_rk3036,
 	pll_rk3066,
+	pll_rk3328,
 	pll_rk3399,
 };
 
-- 
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 related

* [PATCH v1 3/3] clk: rockchip: add new pll-type for rk3328 and similar socs
From: Elaine Zhang @ 2016-12-13  8:47 UTC (permalink / raw)
  To: heiko, mturquette, sboyd, xf
  Cc: robh+dt, mark.rutland, linux-clk, linux-arm-kernel, devicetree,
	huangtao, xxx, cl, linux-rockchip, linux-kernel, Elaine Zhang
In-Reply-To: <1481618869-1239-1-git-send-email-zhangqing@rock-chips.com>

The rk3328's pll and clock are similar with rk3036's,
it different with pll_mode_mask,there are different
adjust foctors and control registers,
so these should be independent and separate from
the series of rk3328s.

Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
---
 drivers/clk/rockchip/clk-pll.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c
index 6ed605776abd..9650c75f61d1 100644
--- a/drivers/clk/rockchip/clk-pll.c
+++ b/drivers/clk/rockchip/clk-pll.c
@@ -29,6 +29,7 @@
 #define PLL_MODE_SLOW		0x0
 #define PLL_MODE_NORM		0x1
 #define PLL_MODE_DEEP		0x2
+#define PLL_RK3328_MODE_MASK	0x1
 
 struct rockchip_clk_pll {
 	struct clk_hw		hw;
@@ -865,13 +866,17 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx,
 	pll_mux = &pll->pll_mux;
 	pll_mux->reg = ctx->reg_base + mode_offset;
 	pll_mux->shift = mode_shift;
-	pll_mux->mask = PLL_MODE_MASK;
+	if (pll_type == pll_rk3328)
+		pll_mux->mask = PLL_RK3328_MODE_MASK;
+	else
+		pll_mux->mask = PLL_MODE_MASK;
 	pll_mux->flags = 0;
 	pll_mux->lock = &ctx->lock;
 	pll_mux->hw.init = &init;
 
 	if (pll_type == pll_rk3036 ||
 	    pll_type == pll_rk3066 ||
+	    pll_type == pll_rk3328 ||
 	    pll_type == pll_rk3399)
 		pll_mux->flags |= CLK_MUX_HIWORD_MASK;
 
@@ -929,6 +934,12 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx,
 		else
 			init.ops = &rockchip_rk3066_pll_clk_ops;
 		break;
+	case pll_rk3328:
+		if (!pll->rate_table || IS_ERR(ctx->grf))
+			init.ops = &rockchip_rk3036_pll_clk_norate_ops;
+		else
+			init.ops = &rockchip_rk3036_pll_clk_ops;
+		break;
 	case pll_rk3399:
 		if (!pll->rate_table)
 			init.ops = &rockchip_rk3399_pll_clk_norate_ops;
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 3/3] clk: keystone: Add sci-clk driver support
From: Tero Kristo @ 2016-12-13  9:01 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-clk-u79uwXL29TY76Z2rM5mHXA,
	mturquette-rdvid1DuHRBWk0Htik3J/w,
	ssantosh-DgEjT+Ai2ygdnm+yROfE0A, nm-l0cyMroinI0,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161212193800.GL5423-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

On 12/12/16 21:38, Stephen Boyd wrote:
> On 12/09, Tero Kristo wrote:
>> On 08/12/16 23:10, Stephen Boyd wrote:
>>> On 12/08, Tero Kristo wrote:
>>>> On 08/12/16 02:13, Stephen Boyd wrote:
>>>>> On 10/21, Tero Kristo wrote:
>>>>>> diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c
>>>>>> new file mode 100644
>>>>>> index 0000000..f6af5bd
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/clk/keystone/sci-clk.c
>>>>
>>>>>
>>>>>> +
>>>>>> +	handle = devm_ti_sci_get_handle(dev);
>>>>>> +	if (IS_ERR(handle))
>>>>>> +		return PTR_ERR(handle);
>>>>>> +
>>>>>> +	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
>>>>>> +	if (!provider)
>>>>>> +		return -ENOMEM;
>>>>>> +
>>>>>> +	provider->clocks = data;
>>>>>> +
>>>>>> +	provider->sci = handle;
>>>>>> +	provider->ops = &handle->ops.clk_ops;
>>>>>> +	provider->dev = dev;
>>>>>> +
>>>>>> +	ti_sci_init_clocks(provider);
>>>>>
>>>>> And if this fails?
>>>>
>>>> Yea this is kind of controversial. ti_sci_init_clocks() can fail if
>>>> any of the clocks registered will fail. I decided to have it this
>>>> way so that at least some clocks might work in failure cause, and
>>>> you might have a booting device instead of total lock-up.
>>>>
>>>> Obviously it could be done so that if any clock fails, we would
>>>> de-register all clocks at that point, but personally I think this is
>>>> a worse option.
>>>>
>>>> ti_sci_init_clocks could probably be modified to continue
>>>> registering clocks when a single clock fails though. Currently it
>>>> aborts at first failure.
>>>>
>>>
>>> That sounds like a better approach if we don't care about
>>> failures to register a clock. Returning a value from a function
>>> and not using it isn't really a great design.
>>>
>>> I worry that if we start returning errors from clk_hw_register()
>>> that something will go wrong though, so really I don't know why
>>> we want to ignore errors at all. Just for debugging a boot hang?
>>> Can't we use early console to at least see that this driver is
>>> failing to probe and debug that way?
>>
>> Early console can be used to debug that, but it is kind of annoying
>> to recompile most of the kernel when you suddenly need to use it.
>
> I thought SERIAL_EARLYCON was selected by drivers that support
> it? So there shouldn't be any rebuilding required.

Actually you can probably ignore my comment, I was just speaking out of 
OMAP experience where the clocks are initialized very early, but this 
doesn't apply to keystone. Sci-clock is a proper driver now with proper 
probe etc. in place so my comment here is invalid.

>
>>
>> How about modifying the ti_sci_init_clocks func to print an error
>> for each failed clock?
>
> Ok that's fine too. I'd prefer the function had a return type of
> void if we're not planning on using the return value, that's all.

Ok, I'll see which way I go in v2 of the series, but seems I can pick 
either your original proposal or mine.

-Tero

--
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 v6 7/8] ARM: dts: stm32: add Timers driver for stm32f429 MCU
From: Benjamin Gaignard @ 2016-12-13  9:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lee Jones, Mark Rutland, alexandre.torgue-qxv4g6HH51o,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	Jonathan Cameron, knaack.h-Mmb7MZpHnFY, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
	Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <20161212185943.ph7njaqb2lxtgdn4@rob-hp-laptop>

2016-12-12 19:59 GMT+01:00 Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> On Fri, Dec 09, 2016 at 03:15:18PM +0100, Benjamin Gaignard wrote:
>> Add Timers and it sub-nodes into DT for stm32f429 family.
>>
>> version 6:
>> - split patch in two: one for SoC family and one for stm32f469
>>   discovery board.
>>
>> version 5:
>> - rename gptimer node to timers
>> - re-order timers node par addresses
>>
>> version 4:
>> - remove unwanted indexing in pwm@ and timer@ node name
>> - use "reg" instead of additional parameters to set timer
>>   configuration
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
>> ---
>>  arch/arm/boot/dts/stm32f429.dtsi | 275 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 275 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..d0fb9cc 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -355,6 +355,21 @@
>>                                       slew-rate = <2>;
>>                               };
>>                       };
>> +
>> +                     pwm1_pins: pwm@1 {
>
> No reg prop, so should not have a unit-address. Given the names in the
> define below, seems like "timer1" would be appropriate.
>
>> +                             pins {
>> +                                     pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> +                                              <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> +                                              <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> +                             };
>> +                     };
>> +
>> +                     pwm3_pins: pwm@3 {
>> +                             pins {
>> +                                     pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> +                                              <STM32F429_PB5_FUNC_TIM3_CH2>;
>> +                             };
>> +                     };
>>               };
>>
>>               rcc: rcc@40023810 {
>> @@ -426,6 +441,266 @@
>>                       interrupts = <80>;
>>                       clocks = <&rcc 0 38>;
>>               };
>> +
>> +             timers2: timers@40000000 {
>
> timer@...
>
> It may be more than just a timer, there's not a better generic name.
>
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40000000 0x400>;
>> +                     clocks = <&rcc 0 128>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <1>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers3: timers@40000400 {
>
> ditto
>
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40000400 0x400>;
>> +                     clocks = <&rcc 0 129>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <2>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers4: timers@40000800 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40000800 0x400>;
>> +                     clocks = <&rcc 0 130>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <3>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers5: timers@40000C00 {
>
> timer@...
>
> And use lowercase hex.
>
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40000C00 0x400>;
>
> ditto
>
>> +                     clocks = <&rcc 0 131>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <4>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers6: timers@40001000 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40001000 0x400>;
>> +                     clocks = <&rcc 0 132>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <5>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers7: timers@40001400 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40001400 0x400>;
>> +                     clocks = <&rcc 0 133>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <6>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers12: timers@40001800 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40001800 0x400>;
>> +                     clocks = <&rcc 0 134>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <9>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers13: timers@40001C00 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40001C00 0x400>;
>> +                     clocks = <&rcc 0 135>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers14: timers@40002000 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40002000 0x400>;
>> +                     clocks = <&rcc 0 136>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers1: timers@40010000 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40010000 0x400>;
>> +                     clocks = <&rcc 0 160>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <0>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers8: timers@40010400 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40010400 0x400>;
>> +                     clocks = <&rcc 0 161>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <7>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers9: timers@40014000 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40014000 0x400>;
>> +                     clocks = <&rcc 0 176>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <8>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers10: timers@40014400 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40014400 0x400>;
>> +                     clocks = <&rcc 0 177>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers11: timers@40014800 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40014800 0x400>;
>> +                     clocks = <&rcc 0 178>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +             };
>>       };
>>  };
>>
>> --
>> 1.9.1
>>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH v2 3/4] dt-bindings: phy: Add support for QMP phy
From: Vivek Gautam @ 2016-12-13  9:18 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: kishon, robh+dt, Mark Rutland, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Srinivas Kandagatla, linux-arm-msm
In-Reply-To: <20161128231925.GO6095@codeaurora.org>

Hi Stephen,

On Tue, Nov 29, 2016 at 4:49 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/22, Vivek Gautam wrote:
>> Qualcomm chipsets have QMP phy controller that provides
>> support to a number of controller, viz. PCIe, UFS, and USB.
>> Adding dt binding information for the same.
>>
>> Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>
>> Changes since v1:
>>  - New patch, forked out of the original driver patch:
>>    "phy: qcom-qmp: new qmp phy driver for qcom-chipsets"
>>  - updated bindings to include mem resource as a list of
>>    offset - length pair for serdes block and for each lane.
>>  - added a new binding for 'lane-offsets' that contains offsets
>>    to tx, rx and pcs blocks from each lane base address.
>>
>>  .../devicetree/bindings/phy/qcom-qmp-phy.txt       | 74 ++++++++++++++++++++++
>>  1 file changed, 74 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>>
>> diff --git a/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> new file mode 100644
>> index 0000000..ffb173b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/qcom-qmp-phy.txt
>> @@ -0,0 +1,74 @@
>> +Qualcomm QMP PHY
>> +----------------
>> +
>> +QMP phy controller supports physical layer functionality for a number of
>> +controllers on Qualcomm chipsets, such as, PCIe, UFS, and USB.

[...]

>> +Example:
>> +     pcie_phy: pciephy@34000 {
>> +             compatible = "qcom,msm8996-qmp-pcie-phy";
>> +             reg = <0x034000 0x48f>,
>> +                     <0x035000 0x5bf>,
>> +                     <0x036000 0x5bf>,
>> +                     <0x037000 0x5bf>;
>> +                             /* tx, rx, pcs */
>> +             lane-offsets = <0x0 0x200 0x400>;
>> +             #phy-cells = <1>;
>> +
>> +             clocks = <&gcc GCC_PCIE_PHY_AUX_CLK>,
>> +                     <&gcc GCC_PCIE_PHY_CFG_AHB_CLK>,
>> +                     <&rpmcc MSM8996_RPM_SMD_LN_BB_CLK>,
>> +                     <&gcc GCC_PCIE_CLKREF_CLK>,
>> +                     <&gcc GCC_PCIE_0_PIPE_CLK>,
>> +                     <&gcc GCC_PCIE_1_PIPE_CLK>,
>> +                     <&gcc GCC_PCIE_2_PIPE_CLK>;
>> +             clock-names = "aux", "cfg_ahb",
>> +                             "ref_clk_src", "ref_clk",
>
> Does MSM8996_RPM_SMD_LN_BB_CLK supply the clock source for
> GCC_PCIE_CLKREF_CLK? Did we mess up the parent/child relationship
> in the GCC driver? We may want to fix that so that this node
> only references clocks that actually go into the device, instead
> of clock parents.

The clock documentations do show that the RPM_SMD_LN_BB_CLK provides
the 19.2 MHz phy clocks via pad. So, like you rightly said we may have to
fix the parents for phy reference clocks for difference phy controllers on 8996.

I will gather some more info on this to discuss it further.


Thanks
Vivek

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply

* Re: [PATCH v6 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2016-12-13  9:20 UTC (permalink / raw)
  To: M'boumba Cedric Madianga
  Cc: wsa, robh+dt, mcoquelin.stm32, alexandre.torgue, linus.walleij,
	patrice.chotard, linux, linux-i2c, devicetree, linux-arm-kernel,
	linux-kernel
In-Reply-To: <1481559342-6106-3-git-send-email-cedric.madianga@gmail.com>

Hello,

On Mon, Dec 12, 2016 at 05:15:39PM +0100, M'boumba Cedric Madianga wrote:
> This patch adds support for the STM32F4 I2C controller.
> 
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> ---
>  drivers/i2c/busses/Kconfig       |  10 +
>  drivers/i2c/busses/Makefile      |   1 +
>  drivers/i2c/busses/i2c-stm32f4.c | 849 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 860 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-stm32f4.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 0cdc844..2719208 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -886,6 +886,16 @@ config I2C_ST
>  	  This driver can also be built as module. If so, the module
>  	  will be called i2c-st.
>  
> +config I2C_STM32F4
> +	tristate "STMicroelectronics STM32F4 I2C support"
> +	depends on ARCH_STM32 || COMPILE_TEST
> +	help
> +	  Enable this option to add support for STM32 I2C controller embedded
> +	  in STM32F4 SoCs.
> +
> +	  This driver can also be built as module. If so, the module
> +	  will be called i2c-stm32f4.
> +
>  config I2C_STU300
>  	tristate "ST Microelectronics DDC I2C interface"
>  	depends on MACH_U300
> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> index 1c1bac8..a2c6ff5 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -85,6 +85,7 @@ obj-$(CONFIG_I2C_SH_MOBILE)	+= i2c-sh_mobile.o
>  obj-$(CONFIG_I2C_SIMTEC)	+= i2c-simtec.o
>  obj-$(CONFIG_I2C_SIRF)		+= i2c-sirf.o
>  obj-$(CONFIG_I2C_ST)		+= i2c-st.o
> +obj-$(CONFIG_I2C_STM32F4)	+= i2c-stm32f4.o
>  obj-$(CONFIG_I2C_STU300)	+= i2c-stu300.o
>  obj-$(CONFIG_I2C_SUN6I_P2WI)	+= i2c-sun6i-p2wi.o
>  obj-$(CONFIG_I2C_TEGRA)		+= i2c-tegra.o
> diff --git a/drivers/i2c/busses/i2c-stm32f4.c b/drivers/i2c/busses/i2c-stm32f4.c
> new file mode 100644
> index 0000000..89ad579
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-stm32f4.c
> @@ -0,0 +1,849 @@
> +/*
> + * Driver for STMicroelectronics STM32 I2C controller
> + *
> + * Copyright (C) M'boumba Cedric Madianga 2015
> + * Author: M'boumba Cedric Madianga <cedric.madianga@gmail.com>
> + *
> + * This driver is based on i2c-st.c
> + *
> + * License terms:  GNU General Public License (GPL), version 2
> + */

If there is a public description available for the device, a link here
would be great.

> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +/* STM32F4 I2C offset registers */
> +#define STM32F4_I2C_CR1			0x00
> +#define STM32F4_I2C_CR2			0x04
> +#define STM32F4_I2C_DR			0x10
> +#define STM32F4_I2C_SR1			0x14
> +#define STM32F4_I2C_SR2			0x18
> +#define STM32F4_I2C_CCR			0x1C
> +#define STM32F4_I2C_TRISE		0x20
> +#define STM32F4_I2C_FLTR		0x24
> +
> +/* STM32F4 I2C control 1*/
> +#define STM32F4_I2C_CR1_SWRST		BIT(15)
> +#define STM32F4_I2C_CR1_POS		BIT(11)
> +#define STM32F4_I2C_CR1_ACK		BIT(10)
> +#define STM32F4_I2C_CR1_STOP		BIT(9)
> +#define STM32F4_I2C_CR1_START		BIT(8)
> +#define STM32F4_I2C_CR1_PE		BIT(0)
> +
> +/* STM32F4 I2C control 2 */
> +#define STM32F4_I2C_CR2_FREQ_MASK	GENMASK(5, 0)
> +#define STM32F4_I2C_CR2_FREQ(n)		((n & STM32F4_I2C_CR2_FREQ_MASK))

This should better be ((n) & STM32F4_I2C_CR2_FREQ_MASK). There a few
more constants that need the same fix.

> +#define STM32F4_I2C_CR2_ITBUFEN		BIT(10)
> +#define STM32F4_I2C_CR2_ITEVTEN		BIT(9)
> +#define STM32F4_I2C_CR2_ITERREN		BIT(8)
> +#define STM32F4_I2C_CR2_IRQ_MASK	(STM32F4_I2C_CR2_ITBUFEN \
> +					| STM32F4_I2C_CR2_ITEVTEN \
> +					| STM32F4_I2C_CR2_ITERREN)

I'd layout this like:

	#define STM32F4_I2C_CR2_IRQ_MASK	(STM32F4_I2C_CR2_ITBUFEN | \
						 STM32F4_I2C_CR2_ITEVTEN | \
						 STM32F4_I2C_CR2_ITERREN)
		
which is more usual I think.

> +/* STM32F4 I2C Status 1 */
> +#define STM32F4_I2C_SR1_AF		BIT(10)
> +#define STM32F4_I2C_SR1_ARLO		BIT(9)
> +#define STM32F4_I2C_SR1_BERR		BIT(8)
> +#define STM32F4_I2C_SR1_TXE		BIT(7)
> +#define STM32F4_I2C_SR1_RXNE		BIT(6)
> +#define STM32F4_I2C_SR1_BTF		BIT(2)
> +#define STM32F4_I2C_SR1_ADDR		BIT(1)
> +#define STM32F4_I2C_SR1_SB		BIT(0)
> +#define STM32F4_I2C_SR1_ITEVTEN_MASK	(STM32F4_I2C_SR1_BTF \
> +					| STM32F4_I2C_SR1_ADDR \
> +					| STM32F4_I2C_SR1_SB)
> +#define STM32F4_I2C_SR1_ITBUFEN_MASK	(STM32F4_I2C_SR1_TXE \
> +					| STM32F4_I2C_SR1_RXNE)
> +#define STM32F4_I2C_SR1_ITERREN_MASK	(STM32F4_I2C_SR1_AF \
> +					| STM32F4_I2C_SR1_ARLO \
> +					| STM32F4_I2C_SR1_BERR)
> +
> +/* STM32F4 I2C Status 2 */
> +#define STM32F4_I2C_SR2_BUSY		BIT(1)
> +
> +/* STM32F4 I2C Control Clock */
> +#define STM32F4_I2C_CCR_CCR_MASK	GENMASK(11, 0)
> +#define STM32F4_I2C_CCR_CCR(n)		((n & STM32F4_I2C_CCR_CCR_MASK))
> +#define STM32F4_I2C_CCR_FS		BIT(15)
> +#define STM32F4_I2C_CCR_DUTY		BIT(14)
> +
> +/* STM32F4 I2C Trise */
> +#define STM32F4_I2C_TRISE_VALUE_MASK	GENMASK(5, 0)
> +#define STM32F4_I2C_TRISE_VALUE(n)	((n & STM32F4_I2C_TRISE_VALUE_MASK))
> +
> +/* STM32F4 I2C Filter */
> +#define STM32F4_I2C_FLTR_DNF_MASK	GENMASK(3, 0)
> +#define STM32F4_I2C_FLTR_DNF(n)		((n & STM32F4_I2C_FLTR_DNF_MASK))
> +#define STM32F4_I2C_FLTR_ANOFF		BIT(4)
> +
> +#define STM32F4_I2C_MIN_FREQ		2U
> +#define STM32F4_I2C_MAX_FREQ		42U
> +#define FAST_MODE_MAX_RISE_TIME		1000
> +#define STD_MODE_MAX_RISE_TIME		300

Are these supposed to be the values "rise time of both SDA and SCL
signals" from the i2c specification? If so, you got it wrong, fast mode
has the smaller value.
Maybe these constants could get a home in a more central place?
Also I'd add /* ns */ to the definition.

> +#define MHZ_TO_HZ			1000000
> +
> +enum stm32f4_i2c_speed {
> +	STM32F4_I2C_SPEED_STANDARD, /* 100 kHz */
> +	STM32F4_I2C_SPEED_FAST, /* 400 kHz */
> +	STM32F4_I2C_SPEED_END,
> +};
> +
> +/**
> + * struct stm32f4_i2c_timings - per-Mode tuning parameters
> + * @duty: Fast mode duty cycle
> + * @mul_ccr: Value to be multiplied to CCR to reach 100Khz/400Khz SCL frequency
> + * @min_ccr: Minimum clock ctrl reg value to reach 100Khz/400Khz SCL frequency
> + */
> +struct stm32f4_i2c_timings {
> +	u32 rate;

rate is undocumented and unused.

> +	u32 duty;
> +	u32 mul_ccr;
> +	u32 min_ccr;
> +};
> +
> +/**
> + * struct stm32f4_i2c_msg - client specific data
> + * @addr: 8-bit slave addr, including r/w bit
> + * @count: number of bytes to be transferred
> + * @buf: data buffer
> + * @result: result of the transfer
> + * @stop: last I2C msg to be sent, i.e. STOP to be generated
> + */
> +struct stm32f4_i2c_msg {
> +	u8	addr;
> +	u32	count;
> +	u8	*buf;
> +	int	result;
> +	bool	stop;
> +};
> +
> +/**
> + * struct stm32f4_i2c_dev - private data of the controller
> + * @adap: I2C adapter for this controller
> + * @dev: device for this controller
> + * @base: virtual memory area
> + * @complete: completion of I2C message
> + * @irq_event: interrupt event line for the controller
> + * @irq_error: interrupt error line for the controller
> + * @clk: hw i2c clock
> + * speed: I2C clock frequency of the controller. Standard or Fast only supported
> + * @msg: I2C transfer information
> + */
> +struct stm32f4_i2c_dev {
> +	struct i2c_adapter		adap;
> +	struct device			*dev;
> +	void __iomem			*base;
> +	struct completion		complete;
> +	int				irq_event;
> +	int				irq_error;

You only use irq_event in the probe function. So there is no need to
remember this one and you could use a local variable instead.

> +	struct clk			*clk;
> +	int				speed;
> +	struct stm32f4_i2c_msg		msg;
> +};
> +
> +static struct stm32f4_i2c_timings i2c_timings[] = {
> +	[STM32F4_I2C_SPEED_STANDARD] = {
> +		.mul_ccr		= 1,
> +		.min_ccr		= 4,
> +		.duty			= 0,
> +	},
> +	[STM32F4_I2C_SPEED_FAST] = {
> +		.mul_ccr		= 16,
> +		.min_ccr		= 1,
> +		.duty			= 1,
> +	},

Are these values from the datasheet?

> +};
> +
> +static inline void stm32f4_i2c_set_bits(void __iomem *reg, u32 mask)
> +{
> +	writel_relaxed(readl_relaxed(reg) | mask, reg);
> +}
> +
> +static inline void stm32f4_i2c_clr_bits(void __iomem *reg, u32 mask)
> +{
> +	writel_relaxed(readl_relaxed(reg) & ~mask, reg);
> +}
> +
> +static void stm32f4_i2c_soft_reset(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
> +
> +	stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_SWRST);
> +	stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_SWRST);

Not very critical, but you're doing an unneeded register access here
because the register is read twice.

Also I think readability would improve if you dropped
stm32f4_i2c_{set,clr}_bits and do their logic explicitly in the callers. 

	stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_SWRST);
	stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_SWRST);

vs

	val = readl_relaxed(reg);
	writel_relaxed(val | STM32F4_I2C_CR1_SWRST, reg);
	writel_relaxed(val, reg);

> +}
> +
> +static void stm32f4_i2c_disable_it(struct stm32f4_i2c_dev *i2c_dev)

What is "it"? If it stands for "interrupt" the more usual abbrev is
"irq".

> +{
> +	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> +
> +	stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_IRQ_MASK);
> +}
> +
> +static void stm32f4_i2c_set_periph_clk_freq(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	u32 clk_rate, cr2, freq;
> +
> +	cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> +	cr2 &= ~STM32F4_I2C_CR2_FREQ_MASK;
> +	clk_rate = clk_get_rate(i2c_dev->clk);
> +	freq = clk_rate / MHZ_TO_HZ;
> +	freq = clamp(freq, STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ);
> +	cr2 |= STM32F4_I2C_CR2_FREQ(freq);
> +	writel_relaxed(cr2, i2c_dev->base + STM32F4_I2C_CR2);

Can you quote the data sheet enough in a comment here to make it obvious
that your calculation is right?

Would it be more sensible to error out if clk_rate / MHZ_TO_HZ isn't in
the interval [STM32F4_I2C_MIN_FREQ, STM32F4_I2C_MAX_FREQ]?

Usually I would expect that you need to use
DIV_ROUND_UP(clk_rate, MHZ_TO_HZ) instead of a plain division.

> +}
> +
> +static void stm32f4_i2c_set_rise_time(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	u32 trise, freq, cr2, val;
> +
> +	cr2 = readl_relaxed(i2c_dev->base + STM32F4_I2C_CR2);
> +	freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
> +
> +	trise = readl_relaxed(i2c_dev->base + STM32F4_I2C_TRISE);
> +	trise &= ~STM32F4_I2C_TRISE_VALUE_MASK;

Are you required to use rmw for STM32F4_I2C_TRISE? I'd prefer

	writel_relaxed(STM32F4_I2C_TRISE_VALUE(..), i2c_dev->base + STM32F4_I2C_TRISE);

unless the datasheet requires rmw.

> +	/* Maximum rise time computation */
> +	if (i2c_dev->speed == STM32F4_I2C_SPEED_STANDARD) {
> +		trise |= STM32F4_I2C_TRISE_VALUE((freq + 1));

A single pair of parenthesis is enough when you fix
STM32F4_I2C_TRISE_VALUE as suggested above.

> +	} else {
> +		val = freq * FAST_MODE_MAX_RISE_TIME / STD_MODE_MAX_RISE_TIME;
> +		trise |= STM32F4_I2C_TRISE_VALUE((val + 1));

val could be local to this branch.

Or make it shorter using:

	freq = cr2 & STM32F4_I2C_CR2_FREQ_MASK;
	if (i2c_dev->speed == STM32F4_I2C_SPEED_FAST)
		freq = freq * FAST_MODE_MAX_RISE_TIME / STD_MODE_MAX_RISE_TIME;

	writel_relaxed(STM32F4_I2C_TRISE_VALUE(freq + 1), ...);

A quote from the data sheet about the algorithm would be good here, too.

> +	}
> +
> +	writel_relaxed(trise, i2c_dev->base + STM32F4_I2C_TRISE);
> +}
> +
> +static void stm32f4_i2c_set_speed_mode(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	struct stm32f4_i2c_timings *t = &i2c_timings[i2c_dev->speed];
> +	u32 ccr, clk_rate;
> +	int val;
> +
> +	ccr = readl_relaxed(i2c_dev->base + STM32F4_I2C_CCR);
> +	ccr &= ~(STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY |
> +		 STM32F4_I2C_CCR_CCR_MASK);
> +
> +	clk_rate = clk_get_rate(i2c_dev->clk);
> +	val = clk_rate / MHZ_TO_HZ * t->mul_ccr;

Is the rounding done right? Again please describe the hardware in a
comment.

> +	if (val < t->min_ccr)
> +		val = t->min_ccr;
> +	ccr |= STM32F4_I2C_CCR_CCR(val);
> +
> +	if (t->duty)
> +		ccr |= STM32F4_I2C_CCR_FS | STM32F4_I2C_CCR_DUTY;
> +
> +	writel_relaxed(ccr, i2c_dev->base + STM32F4_I2C_CCR);
> +}
> +[...]
> +
> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	u32 status;
> +	int ret;
> +
> +	ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
> +					 status,
> +					 !(status & STM32F4_I2C_SR2_BUSY),
> +					 10, 1000);
> +	if (ret) {
> +		dev_err(i2c_dev->dev, "bus not free\n");
> +		ret = -EBUSY;

I'm not sure if "bus not free" deserves an error message. Wolfram?

> +	}
> +
> +	return ret;
> +}
> +
> +[...]
> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> +	u32 rbuf;
> +
> +	rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
> +	*msg->buf++ = (u8)rbuf & 0xff;

unneeded cast (or unneeded & 0xff).

> +	msg->count--;
> +}
> +
> +[...]
> +/**
> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> +	void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> +
> +	switch (msg->count) {
> +	case 1:
> +		stm32f4_i2c_disable_it(i2c_dev);
> +		stm32f4_i2c_read_msg(i2c_dev);
> +		complete(&i2c_dev->complete);
> +		break;
> +	case 2:
> +	case 3:
> +		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> +		break;
> +	default:
> +		stm32f4_i2c_read_msg(i2c_dev);
> +	}

It looks wrong that you don't call stm32f4_i2c_read_msg if msg->count is
2 or 3. I guess that's because these cases are handled in
stm32f4_i2c_handle_rx_btf? Maybe you can simplify the logic a bit?

> +}
> +
> +/**
> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
> + * in case of read
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> +	void __iomem *reg;
> +	u32 mask;
> +	int i;
> +
> +	switch (msg->count) {

I don't understand why the handling depends on the number of messages.

> +	case 2:
> +		reg = i2c_dev->base + STM32F4_I2C_CR1;
> +		/* Generate STOP or REPSTART */

I stumbled about "REPSTART" and would spell it out as "repeated Start".

> +		if (msg->stop)
> +			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> +		else
> +			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> +
> +		/* Read two last data bytes */
> +		for (i = 2; i > 0; i--)
> +			stm32f4_i2c_read_msg(i2c_dev);
> +
> +		/* Disable EVT and ERR interrupt */
> +		reg = i2c_dev->base + STM32F4_I2C_CR2;
> +		mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
> +		stm32f4_i2c_clr_bits(reg, mask);
> +
> +		complete(&i2c_dev->complete);
> +		break;
> +	case 3:
> +		/* Enable ACK and read data */
> +		reg = i2c_dev->base + STM32F4_I2C_CR1;
> +		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> +		stm32f4_i2c_read_msg(i2c_dev);
> +		break;
> +	default:
> +		stm32f4_i2c_read_msg(i2c_dev);
> +	}
> +}
> +
> +/**
> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
> + * master receiver
> + * @i2c_dev: Controller's private data
> + */
> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
> +{
> +	struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> +	void __iomem *reg;
> +
> +	switch (msg->count) {
> +	case 0:
> +		stm32f4_i2c_terminate_xfer(i2c_dev);
> +		/* Clear ADDR flag */
> +		readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> +		break;
> +	case 1:
> +		/*
> +		 * Single byte reception:
> +		 * Enable NACK, clear ADDR flag and generate STOP or RepSTART
> +		 */
> +		reg = i2c_dev->base + STM32F4_I2C_CR1;
> +		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> +		if (msg->stop)
> +			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> +		else
> +			stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> +		break;
> +	case 2:
> +		/*
> +		 * 2-byte reception:
> +		 * Enable NACK and PEC Position Ack and clear ADDR flag

What is PEC?

> +		 */
> +		reg = i2c_dev->base + STM32F4_I2C_CR1;
> +		stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> +		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
> +		readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> +		break;
> +
> +	default:
> +		/* N-byte reception: Enable ACK and clear ADDR flag */
> +		reg = i2c_dev->base + STM32F4_I2C_CR1;
> +		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
> +		readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> +		break;
> +	}
> +}
> +
> +/**
> + * stm32f4_i2c_isr_event() - Interrupt routine for I2C bus event
> + * @irq: interrupt number
> + * @data: Controller's private data
> + */
> +static irqreturn_t stm32f4_i2c_isr_event(int irq, void *data)
> +{
> +[...]
> +	real_status = readl_relaxed(i2c_dev->base + STM32F4_I2C_SR1);

s/real_status/status/ ?

> +
> +	if (!(real_status & possible_status)) {
> +		dev_dbg(i2c_dev->dev,
> +			"spurious evt it (status=0x%08x, ien=0x%08x)\n",
> +			real_status, ien);

s/it/irq/

> +		return IRQ_NONE;
> +	}
> +
> +	/* Use __fls() to check error bits first */
> +	flag = __fls(real_status & possible_status);

If you get several events reported you only handle a single one. Is this
effective?

> +	switch (1 << flag) {
> +	case STM32F4_I2C_SR1_SB:
> +		stm32f4_i2c_write_byte(i2c_dev, msg->addr);
> +		break;
> +
> +	case STM32F4_I2C_SR1_ADDR:
> +		if (msg->addr & I2C_M_RD)
> +			stm32f4_i2c_handle_rx_addr(i2c_dev);
> +		else
> +			readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> +
> +		/* Enable ITBUF interrupts */

What is ITBUF?

> +		reg = i2c_dev->base + STM32F4_I2C_CR2;
> +		stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> +		break;
> +
> +	case STM32F4_I2C_SR1_BTF:
> +		if (msg->addr & I2C_M_RD)
> +			stm32f4_i2c_handle_rx_btf(i2c_dev);
> +		else
> +			stm32f4_i2c_handle_write(i2c_dev);
> +		break;
> +
> +	case STM32F4_I2C_SR1_TXE:
> +		stm32f4_i2c_handle_write(i2c_dev);
> +		break;
> +
> +	case STM32F4_I2C_SR1_RXNE:
> +		stm32f4_i2c_handle_read(i2c_dev);
> +		break;
> +
> +	default:
> +		dev_err(i2c_dev->dev,
> +			"evt it unhandled: status=0x%08x)\n", real_status);

s/it/irq/

> +		return IRQ_NONE;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +[...]
> +static int stm32f4_i2c_xfer_msg(struct stm32f4_i2c_dev *i2c_dev,
> +				struct i2c_msg *msg, bool is_first,
> +				bool is_last)
> +{
> +[...]
> +	/* Enable ITEVT and ITERR interrupts */

This comment isn't helpful. Mentioning their meaning would be great
instead.

> +[...]
> +static int stm32f4_i2c_xfer(struct i2c_adapter *i2c_adap, struct i2c_msg msgs[],
> +			    int num)
> +{
> +	struct stm32f4_i2c_dev *i2c_dev = i2c_get_adapdata(i2c_adap);
> +	int ret, i;
> +
> +	ret = clk_enable(i2c_dev->clk);
> +	if (ret) {
> +		dev_err(i2c_dev->dev, "Failed to enable clock\n");
> +		return ret;
> +	}
> +
> +	stm32f4_i2c_hw_config(i2c_dev);
> +
> +	for (i = 0; i < num && !ret; i++)
> +		ret = stm32f4_i2c_xfer_msg(i2c_dev, &msgs[i], i == 0,
> +					   i == num - 1);
> +
> +	clk_disable(i2c_dev->clk);
> +
> +	return (ret < 0) ? ret : i;

using num instead of i would be a bit more obvious.

> +static int stm32f4_i2c_probe(struct platform_device *pdev)
> +{
> +[...]
> +	i2c_dev->speed = STM32F4_I2C_SPEED_STANDARD;
> +	ret = of_property_read_u32(np, "clock-frequency", &clk_rate);
> +	if ((!ret) && (clk_rate == 400000))
> +		i2c_dev->speed = STM32F4_I2C_SPEED_FAST;

I'd use

	if (!ret && clk_rate >= 400000)
		i2c_dev->speed = STM32F4_I2C_SPEED_FAST;

. That's less parenthesis and a more robust selection of the bus
frequency.

> +
> +	i2c_dev->dev = &pdev->dev;
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_event,
> +					NULL, stm32f4_i2c_isr_event,
> +					IRQF_ONESHOT, pdev->name, i2c_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request irq %i\n",
> +			i2c_dev->irq_error);

That's wrong. Requesting irq_event failed.

> +		goto clk_free;
> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, i2c_dev->irq_error,
> +					NULL, stm32f4_i2c_isr_error,
> +					IRQF_ONESHOT, pdev->name, i2c_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request irq %i\n",
> +			i2c_dev->irq_error);
> +		goto clk_free;

It would also be nice to know for which type of irq this failed. I.e.
please point out if this is the error irq or the event irq in the
message. Ditto for checking the return type of irq_of_parse_and_map.

> +	}
> +
> +	adap = &i2c_dev->adap;
> +	i2c_set_adapdata(adap, i2c_dev);
> +	snprintf(adap->name, sizeof(adap->name), "STM32 I2C(%pa)", &res->start);
> +	adap->owner = THIS_MODULE;
> +	adap->timeout = 2 * HZ;
> +	adap->retries = 0;
> +	adap->algo = &stm32f4_i2c_algo;
> +	adap->dev.parent = &pdev->dev;
> +	adap->dev.of_node = pdev->dev.of_node;
> +
> +	init_completion(&i2c_dev->complete);
> +
> +	ret = i2c_add_adapter(adap);
> +	if (ret)
> +		goto clk_free;
> +
> +	platform_set_drvdata(pdev, i2c_dev);
> +
> +	dev_info(i2c_dev->dev, "STM32F4 I2C driver initialized\n");

This is wrong. The driver is bound now to a device, not initialized.

> +static const struct of_device_id stm32f4_i2c_match[] = {
> +	{ .compatible = "st,stm32f4-i2c", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, stm32f4_i2c_match);
> +
> +static struct platform_driver stm32f4_i2c_driver = {
> +	.driver = {
> +		.name = "stm32f4-i2c",
> +		.of_match_table = stm32f4_i2c_match,

Is this needed?

> +	},
> +	.probe = stm32f4_i2c_probe,
> +	.remove = stm32f4_i2c_remove,
> +};

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH v6 1/8] MFD: add bindings for STM32 Timers driver
From: Benjamin Gaignard @ 2016-12-13  9:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lee Jones, Mark Rutland, alexandre.torgue-qxv4g6HH51o,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
	Thierry Reding, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	Jonathan Cameron, knaack.h-Mmb7MZpHnFY, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
	Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <20161212185149.rt3xqpn3mbaavb4l@rob-hp-laptop>

2016-12-12 19:51 GMT+01:00 Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> On Fri, Dec 09, 2016 at 03:15:12PM +0100, Benjamin Gaignard wrote:
>> Add bindings information for STM32 Timers
>>
>> version 6:
>> - rename stm32-gtimer to stm32-timers
>> - change compatible
>> - add description about the IPs
>>
>> version 2:
>> - rename stm32-mfd-timer to stm32-gptimer
>> - only keep one compatible string
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
>> ---
>>  .../devicetree/bindings/mfd/stm32-timers.txt       | 46 ++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mfd/stm32-timers.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/stm32-timers.txt b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
>> new file mode 100644
>> index 0000000..b30868e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/stm32-timers.txt
>> @@ -0,0 +1,46 @@
>> +STM32 Timers driver bindings
>> +
>> +This IP provides 3 types of timer along with PWM functionality:
>> +- advanced-control timers consist of a 16-bit auto-reload counter driven by a programmable
>> +  prescaler, break input feature, PWM outputs and complementary PWM ouputs channels.
>> +- general-purpose timers consist of a 16-bit or 32-bit auto-reload counter driven by a
>> +  programmable prescaler and PWM outputs.
>> +- basic timers consist of a 16-bit auto-reload counter driven by a programmable prescaler.
>> +
>> +Required parameters:
>> +- compatible: must be "st,stm32-timers"
>> +
>> +- reg:                       Physical base address and length of the controller's
>> +                     registers.
>> +- clock-names:               Set to "clk_int".
>
> 'clk' is redundant. Also, you don't really need -names when there is
> only one of them.

I use devm_regmap_init_mmio_clk() which get the clock by it name so
I have to define it in DT.

>> +- clocks:            Phandle to the clock used by the timer module.
>> +                     For Clk properties, please refer to ../clock/clock-bindings.txt
>> +
>> +Optional parameters:
>> +- resets:            Phandle to the parent reset controller.
>> +                     See ../reset/st,stm32-rcc.txt
>> +
>> +Optional subnodes:
>> +- pwm:                       See ../pwm/pwm-stm32.txt
>> +- timer:             See ../iio/timer/stm32-timer-trigger.txt
>> +
>> +Example:
>> +     timers@40010000 {
>> +             #address-cells = <1>;
>> +             #size-cells = <0>;
>> +             compatible = "st,stm32-timers";
>> +             reg = <0x40010000 0x400>;
>> +             clocks = <&rcc 0 160>;
>> +             clock-names = "clk_int";
>> +
>> +             pwm {
>> +                     compatible = "st,stm32-pwm";
>> +                     pinctrl-0       = <&pwm1_pins>;
>> +                     pinctrl-names   = "default";
>> +             };
>> +
>> +             timer {
>> +                     compatible = "st,stm32-timer-trigger";
>> +                     reg = <0>;
>
> You don't need reg here as there is only one. In turn, you don't need
> #address-cells or #size-cells.

I use "reg" to set each timer configuration.
>From hardware point of view they are all the same except for which hardware
signals they could consume and/or send.
"reg" is used as index of the two tables in driver code.

>
>> +             };
>> +     };
>> --
>> 1.9.1
>>

^ permalink raw reply

* Re: [PATCH v6 7/8] ARM: dts: stm32: add Timers driver for stm32f429 MCU
From: Benjamin Gaignard @ 2016-12-13  9:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Lee Jones, Mark Rutland, alexandre.torgue, devicetree,
	Linux Kernel Mailing List, Thierry Reding, linux-pwm,
	Jonathan Cameron, knaack.h, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio, linux-arm-kernel,
	Fabrice Gasnier, Gerald Baeza, Arnaud Pouliquen, Linus Walleij,
	Linaro Kernel Mailman List, Benjamin Gaignard
In-Reply-To: <20161212185943.ph7njaqb2lxtgdn4@rob-hp-laptop>

2016-12-12 19:59 GMT+01:00 Rob Herring <robh@kernel.org>:
> On Fri, Dec 09, 2016 at 03:15:18PM +0100, Benjamin Gaignard wrote:
>> Add Timers and it sub-nodes into DT for stm32f429 family.
>>
>> version 6:
>> - split patch in two: one for SoC family and one for stm32f469
>>   discovery board.
>>
>> version 5:
>> - rename gptimer node to timers
>> - re-order timers node par addresses
>>
>> version 4:
>> - remove unwanted indexing in pwm@ and timer@ node name
>> - use "reg" instead of additional parameters to set timer
>>   configuration
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>  arch/arm/boot/dts/stm32f429.dtsi | 275 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 275 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..d0fb9cc 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -355,6 +355,21 @@
>>                                       slew-rate = <2>;
>>                               };
>>                       };
>> +
>> +                     pwm1_pins: pwm@1 {
>
> No reg prop, so should not have a unit-address. Given the names in the
> define below, seems like "timer1" would be appropriate.
>

Here pins muxing is only targeting PWM part of the the MFD , that why I have
labeled it with "pwm".

>> +                             pins {
>> +                                     pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> +                                              <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> +                                              <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> +                             };
>> +                     };
>> +
>> +                     pwm3_pins: pwm@3 {
>> +                             pins {
>> +                                     pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> +                                              <STM32F429_PB5_FUNC_TIM3_CH2>;
>> +                             };
>> +                     };
>>               };
>>
>>               rcc: rcc@40023810 {
>> @@ -426,6 +441,266 @@
>>                       interrupts = <80>;
>>                       clocks = <&rcc 0 38>;
>>               };
>> +
>> +             timers2: timers@40000000 {
>
> timer@...
>
> It may be more than just a timer, there's not a better generic name.

"timer" is already used in DT for clocksource driver.
"timers" cover "advanced-control", "generic" and "basic" hardware timers IPs,
which share the same registers mapping (only the level of feature are different)

>
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40000000 0x400>;
>> +                     clocks = <&rcc 0 128>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <1>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers3: timers@40000400 {
>
> ditto
>
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40000400 0x400>;
>> +                     clocks = <&rcc 0 129>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <2>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers4: timers@40000800 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40000800 0x400>;
>> +                     clocks = <&rcc 0 130>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <3>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers5: timers@40000C00 {
>
> timer@...
>
> And use lowercase hex.

ok

>
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40000C00 0x400>;
>
> ditto
>
>> +                     clocks = <&rcc 0 131>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <4>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers6: timers@40001000 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40001000 0x400>;
>> +                     clocks = <&rcc 0 132>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <5>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers7: timers@40001400 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40001400 0x400>;
>> +                     clocks = <&rcc 0 133>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <6>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers12: timers@40001800 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40001800 0x400>;
>> +                     clocks = <&rcc 0 134>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <9>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers13: timers@40001C00 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40001C00 0x400>;
>> +                     clocks = <&rcc 0 135>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers14: timers@40002000 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40002000 0x400>;
>> +                     clocks = <&rcc 0 136>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers1: timers@40010000 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40010000 0x400>;
>> +                     clocks = <&rcc 0 160>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <0>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers8: timers@40010400 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40010400 0x400>;
>> +                     clocks = <&rcc 0 161>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <7>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers9: timers@40014000 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40014000 0x400>;
>> +                     clocks = <&rcc 0 176>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +
>> +                     timer {
>> +                             compatible = "st,stm32-timer-trigger";
>> +                             reg = <8>;
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers10: timers@40014400 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40014400 0x400>;
>> +                     clocks = <&rcc 0 177>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +             };
>> +
>> +             timers11: timers@40014800 {
>> +                     #address-cells = <1>;
>> +                     #size-cells = <0>;
>> +                     compatible = "st,stm32-timers";
>> +                     reg = <0x40014800 0x400>;
>> +                     clocks = <&rcc 0 178>;
>> +                     clock-names = "clk_int";
>> +                     status = "disabled";
>> +
>> +                     pwm {
>> +                             compatible = "st,stm32-pwm";
>> +                             status = "disabled";
>> +                     };
>> +             };
>>       };
>>  };
>>
>> --
>> 1.9.1
>>

^ permalink raw reply

* Re: [PATCH v1 2/3] clk: rockchip: add clock controller for rk3328
From: Shawn Lin @ 2016-12-13  9:38 UTC (permalink / raw)
  To: Elaine Zhang
  Cc: heiko, mturquette, sboyd, xf, shawn.lin, mark.rutland, devicetree,
	huangtao, xxx, linux-kernel, linux-rockchip, robh+dt, cl,
	linux-clk, linux-arm-kernel
In-Reply-To: <1481618869-1239-3-git-send-email-zhangqing@rock-chips.com>

Hi, Elaine,

I always only keep an eye for mmc stuff here. :)

On 2016/12/13 16:47, Elaine Zhang wrote:
> Add the clock tree definition for the new rk3328 SoC.
>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
> ---

----8<--------------

> +
> +	/* PD_MMC */
> +	MMC(SCLK_SDMMC_DRV, "sdmmc_drv", "sclk_sdmmc",
> +	    RK3328_SDMMC_CON0, 1),
> +	MMC(SCLK_SDMMC_SAMPLE, "sdmmc_sample", "sclk_sdmmc",
> +	    RK3328_SDMMC_CON1, 0),
> +

All of offset for these *_sample are wrong, and they should be 1
, the same as *_drv. You could refer to P565 of TRM instead the
section of CRU for these details.

> +	MMC(SCLK_SDIO_DRV, "sdio_drv", "sclk_sdio",
> +	    RK3328_SDIO_CON0, 1),
> +	MMC(SCLK_SDIO_SAMPLE, "sdio_sample", "sclk_sdio",
> +	    RK3328_SDIO_CON1, 0),
> +
> +	MMC(SCLK_EMMC_DRV, "emmc_drv", "sclk_emmc",
> +	    RK3328_EMMC_CON0, 1),
> +	MMC(SCLK_EMMC_SAMPLE, "emmc_sample", "sclk_emmc",
> +	    RK3328_EMMC_CON1, 0),
> +
> +	MMC(SCLK_SDMMC_EXT_DRV, "sdmmc_ext_drv", "sclk_sdmmc_ext",
> +	    RK3328_SDMMC_EXT_CON0, 1),
> +	MMC(SCLK_SDMMC_EXT_SAMPLE, "sdmmc_ext_sample", "sclk_sdmmc_ext",
> +	    RK3328_SDMMC_EXT_CON1, 0),
> +};
> +

----8<--------


> +#define RK3328_SDMMC_CON0		0x380
> +#define RK3328_SDMMC_CON1		0x384
> +#define RK3328_SDIO_CON0		0x388
> +#define RK3328_SDIO_CON1		0x38c
> +#define RK3328_EMMC_CON0		0x390
> +#define RK3328_EMMC_CON1		0x394
> +#define RK3328_SDMMC_EXT_CON0		0x398
> +#define RK3328_SDMMC_EXT_CON1		0x39C


Just wondering is it worth, but this uppercase 'C' isn't
consistent with the former lowercase


> +
>  #define RK3368_PLL_CON(x)		RK2928_PLL_CON(x)
>  #define RK3368_CLKSEL_CON(x)		((x) * 0x4 + 0x100)
>  #define RK3368_CLKGATE_CON(x)		((x) * 0x4 + 0x200)
> @@ -130,6 +152,7 @@
>  enum rockchip_pll_type {
>  	pll_rk3036,
>  	pll_rk3066,
> +	pll_rk3328,
>  	pll_rk3399,
>  };
>
>


-- 
Best Regards
Shawn Lin

^ permalink raw reply

* Re: [PATCH v6 3/5] ARM: dts: stm32: Add I2C1 support for STM32F429 SoC
From: Alexandre Torgue @ 2016-12-13 10:01 UTC (permalink / raw)
  To: M'boumba Cedric Madianga, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A, patrice.chotard-qxv4g6HH51o,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1481559342-6106-4-git-send-email-cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Hi Cedric,

On 12/12/2016 05:15 PM, M'boumba Cedric Madianga wrote:
> Signed-off-by: Patrice Chotard <patrice.chotard-qxv4g6HH51o@public.gmane.org>
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Please Add a commit message.

> ---
>  arch/arm/boot/dts/stm32f429.dtsi | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index 7de52ee..cbdece7 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -48,6 +48,7 @@
>  #include "skeleton.dtsi"
>  #include "armv7-m.dtsi"
>  #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
> +#include <dt-bindings/mfd/stm32f4-rcc.h>
>
>  / {
>  	clocks {
> @@ -337,6 +338,16 @@
>  					slew-rate = <2>;
>  				};
>  			};
> +
> +			i2c1_pins_b: i2c1@0 {
> +				pins1 {
> +					pinmux = <STM32F429_PB9_FUNC_I2C1_SDA>;
> +					drive-open-drain;
> +				};
> +				pins2 {
> +					pinmux = <STM32F429_PB6_FUNC_I2C1_SCL>;
> +				};
> +			};
>  		};
>
>  		rcc: rcc@40023810 {
> @@ -409,6 +420,18 @@
>  			interrupts = <80>;
>  			clocks = <&rcc 0 38>;
>  		};
> +
> +		i2c1: i2c@40005400 {

Can you check the order on device node please ? (should follow base@)

> +			compatible = "st,stm32f4-i2c";
> +			reg = <0x40005400 0x400>;
> +			interrupts = <31>,
> +				     <32>;
> +			resets = <&rcc STM32F4_APB1_RESET(I2C1)>;
> +			clocks = <&rcc 0 STM32F4_APB1_CLOCK(I2C1)>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			status = "disabled";
> +		};
>  	};
>  };
>
>
--
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 v6 4/5] ARM: dts: stm32: Add I2C1 support for STM32429 eval board
From: Alexandre Torgue @ 2016-12-13 10:02 UTC (permalink / raw)
  To: M'boumba Cedric Madianga, wsa, robh+dt, mcoquelin.stm32,
	linus.walleij, patrice.chotard, linux, linux-i2c, devicetree,
	linux-arm-kernel, linux-kernel
In-Reply-To: <1481559342-6106-5-git-send-email-cedric.madianga@gmail.com>

Hi Cedric,

On 12/12/2016 05:15 PM, M'boumba Cedric Madianga wrote:
> Signed-off-by: M'boumba Cedric Madianga <cedric.madianga@gmail.com>

Can you please add a commit message.

> ---
>  arch/arm/boot/dts/stm32429i-eval.dts | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
> index afb90bc..74e0045 100644
> --- a/arch/arm/boot/dts/stm32429i-eval.dts
> +++ b/arch/arm/boot/dts/stm32429i-eval.dts
> @@ -141,3 +141,9 @@
>  	pinctrl-names = "default";
>  	status = "okay";
>  };
> +
> +&i2c1 {
> +	pinctrl-0 = <&i2c1_pins_b>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +};
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox