* Re: [PATCH v2 2/4] phy: qcom-qusb2: New driver for QUSB2 PHY on Qcom chips
From: Vivek Gautam @ 2016-12-01 8:42 UTC (permalink / raw)
To: Stephen Boyd
Cc: kishon, robh+dt, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Srinivas Kandagatla, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161128231424.GN6095-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Hi Stephen,
On Tue, Nov 29, 2016 at 4:44 AM, Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> On 11/22, Vivek Gautam wrote:
>>
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index e8eb7f2..f1dcec1 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -430,6 +430,17 @@ config PHY_STIH407_USB
>> Enable this support to enable the picoPHY device used by USB2
>> and USB3 controllers on STMicroelectronics STiH407 SoC families.
>>
>> +config PHY_QCOM_QUSB2
>> + tristate "Qualcomm QUSB2 PHY Driver"
>> + depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> + select GENERIC_PHY
>> + select RESET_CONTROLLER
>
> This shouldn't be necessary. We only need to select it if we're
> providing resets.
Ok, will drop this.
>
>> + help
>> + Enable this to support the HighSpeed QUSB2 PHY transceiver for USB
>> + controllers on Qualcomm chips. This driver supports the high-speed
>> + PHY which is usually paired with either the ChipIdea or Synopsys DWC3
>> + USB IPs on MSM SOCs.
>> +
>> config PHY_QCOM_UFS
>> tristate "Qualcomm UFS PHY driver"
>> depends on OF && ARCH_QCOM
>> diff --git a/drivers/phy/phy-qcom-qusb2.c b/drivers/phy/phy-qcom-qusb2.c
>> new file mode 100644
>> index 0000000..d3f9657
>> --- /dev/null
>> +++ b/drivers/phy/phy-qcom-qusb2.c
>> @@ -0,0 +1,549 @@
>> +/*
>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/nvmem-consumer.h>
>> +#include <linux/of.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include <linux/slab.h>
>> +
>> +#define QUSB2PHY_PLL_TEST 0x04
>> +#define CLK_REF_SEL BIT(7)
>> +
>> +#define QUSB2PHY_PLL_TUNE 0x08
>> +#define QUSB2PHY_PLL_USER_CTL1 0x0c
>> +#define QUSB2PHY_PLL_USER_CTL2 0x10
>> +#define QUSB2PHY_PLL_AUTOPGM_CTL1 0x1c
>> +#define QUSB2PHY_PLL_PWR_CTRL 0x18
>> +
>> +#define QUSB2PHY_PLL_STATUS 0x38
>> +#define PLL_LOCKED BIT(5)
>> +
>> +#define QUSB2PHY_PORT_TUNE1 0x80
>> +#define QUSB2PHY_PORT_TUNE2 0x84
>> +#define QUSB2PHY_PORT_TUNE3 0x88
>> +#define QUSB2PHY_PORT_TUNE4 0x8C
>> +#define QUSB2PHY_PORT_TUNE5 0x90
>> +#define QUSB2PHY_PORT_TEST2 0x9c
>
> Please use lowercase or uppercase consistently (I'd prefer
> lowercase).
Sure, lowercase.
>
>> +
>> +#define QUSB2PHY_PORT_POWERDOWN 0xB4
>> +#define CLAMP_N_EN BIT(5)
>> +#define FREEZIO_N BIT(1)
>> +#define POWER_DOWN BIT(0)
>> +
>> +#define QUSB2PHY_REFCLK_ENABLE BIT(0)
>> +
>> +#define PHY_CLK_SCHEME_SEL BIT(0)
>> +
>> +struct qusb2_phy_init_tbl {
>> + unsigned int reg_offset;
>> + unsigned int cfg_val;
>> +};
>> +#define QCOM_QUSB2_PHY_INIT_CFG(reg, val) \
>> + { \
>> + .reg_offset = reg, \
>> + .cfg_val = val, \
>> + }
>> +
>> +static struct qusb2_phy_init_tbl msm8996_phy_init_tbl[] = {
>
> const?
argh! shouldn't have missed these 'const' and 'static' assignments.
will update for all instances.
>
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE1, 0xF8),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE2, 0xB3),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE3, 0x83),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TUNE4, 0xC0),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL2, 0x21),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PORT_TEST2, 0x14),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_AUTOPGM_CTL1, 0x9F),
>> + QCOM_QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_PWR_CTRL, 0x00),
>> +};
>> +
>> +struct qusb2_phy_init_cfg {
>> + struct qusb2_phy_init_tbl *phy_init_tbl;
>> + int phy_init_tbl_sz;
>> + /* offset to PHY_CLK_SCHEME register in TCSR map. */
>> + unsigned int clk_scheme_offset;
>> +};
>> +
>> +const struct qusb2_phy_init_cfg msm8996_phy_init_cfg = {
>
> static?
>
>> + .phy_init_tbl = msm8996_phy_init_tbl,
>> + .phy_init_tbl_sz = ARRAY_SIZE(msm8996_phy_init_tbl),
>> +};
>> +
>> +/**
>> + * struct qusb2_phy: Structure holding qusb2 phy attributes.
>> + *
>> + * @phy: pointer to generic phy.
>> + * @base: pointer to iomapped memory space for qubs2 phy.
>> + *
>> + * @cfg_ahb_clk: pointer to AHB2PHY interface clock.
>> + * @ref_clk: pointer to reference clock.
>> + * @ref_clk_src: pointer to source to reference clock.
>> + * @iface_src: pointer to phy interface clock.
>> + *
>> + * @phy_reset: Pointer to phy reset control
>> + *
>> + * @vdda_phy: vdd supply to the phy core block.
>> + * @vdda_pll: 1.8V vdd supply to ref_clk block.
>> + * @vdda_phy_dpdm: 3.1V vdd supply to Dp/Dm port signals.
>> + * @tcsr: pointer to TCSR syscon register map.
>
> Drop all the full stops on these comments please.
sure.
>
>> + *
>> + * @cfg: phy initialization config data
>> + * @has_se_clk_scheme: indicate if PHY has Single-ended ref clock scheme
>
> Why is single capitalized?
ok, 'single-ended'
>
>> + */
>> +struct qusb2_phy {
>> + struct phy *phy;
>> + void __iomem *base;
>> +
>> + struct clk *cfg_ahb_clk;
>> + struct clk *ref_clk;
>> + struct clk *ref_clk_src;
>> + struct clk *iface_clk;
>> +
>> + struct reset_control *phy_reset;
>> +
>> + struct regulator *vdd_phy;
>> + struct regulator *vdda_pll;
>> + struct regulator *vdda_phy_dpdm;
>> +
>> + struct regmap *tcsr;
>> +
>> + const struct qusb2_phy_init_cfg *cfg;
>> + bool has_se_clk_scheme;
>> +};
>> +
>> +static inline void qusb2_setbits(void __iomem *reg, u32 val)
>> +{
>> + u32 reg_val;
>> +
>> + reg_val = readl_relaxed(reg);
>> + reg_val |= val;
>> + writel_relaxed(reg_val, reg);
>> +
>> + /* Ensure above write is completed */
>> + mb();
>> +}
>> +
>> +static inline void qusb2_clrbits(void __iomem *reg, u32 val)
>> +{
>> + u32 reg_val;
>> +
>> + reg_val = readl_relaxed(reg);
>> + reg_val &= ~val;
>> + writel_relaxed(reg_val, reg);
>> +
>> + /* Ensure above write is completed */
>> + mb();
>> +}
>> +
>> +static void qcom_qusb2_phy_configure(void __iomem *base,
>> + struct qusb2_phy_init_tbl init_tbl[],
>> + int init_tbl_sz)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < init_tbl_sz; i++) {
>> + writel_relaxed(init_tbl[i].cfg_val,
>> + base + init_tbl[i].reg_offset);
>> + }
>> +
>> + /* flush buffered writes */
>> + mb();
>> +}
>> +
>> +static void qusb2_phy_enable_clocks(struct qusb2_phy *qphy, bool on)
>
> Maybe s/enable/toggle/ because we're not always enabling.
yea, toggle is better; will modify.
>
>> +{
>> + if (on) {
>> + clk_prepare_enable(qphy->iface_clk);
>> + clk_prepare_enable(qphy->ref_clk_src);
>> + } else {
>> + clk_disable_unprepare(qphy->ref_clk_src);
>> + clk_disable_unprepare(qphy->iface_clk);
>> + }
>> +
>> + dev_vdbg(&qphy->phy->dev, "%s(): clocks enabled\n", __func__);
>
> Heh or disabled!
yup, a check for 'on' and relevant string - enabled/disabled. will do it.
>
>> +}
>> +
>> +static int qusb2_phy_enable_power(struct qusb2_phy *qphy, bool on)
>
> Maybe s/enable/toggle/ because we're not always enabling.
Yea, will update it to 'toggle'.
>
>> +{
>> + int ret;
>> + struct device *dev = &qphy->phy->dev;
>> +
>> + if (!on)
>> + goto disable_vdda_phy_dpdm;
>> +
>> + ret = regulator_enable(qphy->vdd_phy);
>> + if (ret) {
>> + dev_err(dev, "Unable to enable vdd-phy:%d\n", ret);
>> + goto err_vdd_phy;
>> + }
>> +
>> + ret = regulator_enable(qphy->vdda_pll);
>> + if (ret) {
>> + dev_err(dev, "Unable to enable vdda-pll:%d\n", ret);
>> + goto disable_vdd_phy;
>> + }
>> +
>> + ret = regulator_enable(qphy->vdda_phy_dpdm);
>> + if (ret) {
>> + dev_err(dev, "Unable to enable vdda-phy-dpdm:%d\n", ret);
>> + goto disable_vdda_pll;
>> + }
>> +
>> + dev_vdbg(dev, "%s() regulators are turned on.\n", __func__);
>
> Drop the full stop please.
ok.
>
>> +
>> + return ret;
>> +
>> +disable_vdda_phy_dpdm:
>> + regulator_disable(qphy->vdda_phy_dpdm);
>> +disable_vdda_pll:
>> + regulator_disable(qphy->vdda_pll);
>> +disable_vdd_phy:
>> + regulator_disable(qphy->vdd_phy);
>> +err_vdd_phy:
>> + dev_vdbg(dev, "%s() regulators are turned off.\n", __func__);
>> + return ret;
>> +}
>> +
>> +/*
>> + * Fetches HS Tx tuning value from e-fuse and sets QUSB2PHY_PORT_TUNE2
>> + * register.
>> + * For any error case, skip setting the value and use the default value.
>> + */
>> +static int qusb2_phy_set_tune2_param(struct qusb2_phy *qphy)
>> +{
>> + struct device *dev = &qphy->phy->dev;
>> + struct nvmem_cell *cell;
>> + ssize_t len;
>> + u8 *val;
>> +
>> + /*
>> + * Read EFUSE register having TUNE2 parameter's high nibble.
>> + * If efuse register shows value as 0x0, or if we fail to find
>> + * a valid efuse register settings, then use default value
>> + * as 0xB for high nibble that we have already set while
>> + * configuring phy.
>> + */
>> + cell = devm_nvmem_cell_get(dev, "tune2_hstx_trim_efuse");
>> + if (IS_ERR(cell)) {
>> + if (PTR_ERR(cell) == -EPROBE_DEFER)
>> + return PTR_ERR(cell);
>> + goto skip;
>
> Why do we get the nvmem cell here? Wouldn't we want to get it
> during probe? Returning probe defer here during init would be
> odd.
Yea, my bad. This should be moved to probe().
>
>> + }
>> +
>> + /*
>> + * we need to read only one byte here, since the required
>> + * parameter value fits in one nibble
>> + */
>> + val = (u8 *)nvmem_cell_read(cell, &len);
>
> Shouldn't need the cast here. Also it would be nice if
> nvmem_cell_read() didn't require a second argument if we don't
> care for it. We should update the API to allow NULL there.
Will remove the u8 pointer cast.
Correct, it makes sense to allow the length pointer to be passed as NULL.
We don't care about this length. Will update the nvmem API, to allow this.
Also, we should add a check for 'cell' as well. This pointer can be
NULL, and the first thing that nvmem_cell_read does is - deference
the pointer 'cell'
>
>> + if (!IS_ERR(val)) {
>> + /* Fused TUNE2 value is the higher nibble only */
>> + qusb2_setbits(qphy->base + QUSB2PHY_PORT_TUNE2,
>> + val[0] << 0x4);
>> + } else {
>> + dev_dbg(dev, "failed reading hs-tx trim value: %ld\n",
>> + PTR_ERR(val));
>> + }
>> +
>> +skip:
>> + return 0;
>> +}
>> +
> [...]
>> +
>> +static int qusb2_phy_init(struct phy *phy)
>> +{
>> + struct qusb2_phy *qphy = phy_get_drvdata(phy);
>> + unsigned int reset_val;
>> + unsigned int clk_scheme;
>> + int ret;
>> +
>> + dev_vdbg(&phy->dev, "Initializing QUSB2 phy\n");
>> +
>> + /* enable ahb interface clock to program phy */
>> + clk_prepare_enable(qphy->cfg_ahb_clk);
>
> What if that fails?
Yea, will add the necessary checks for failure here and in the rest of the patch
wherever necessary.
>
>> +
>> + /* Perform phy reset */
>> + ret = reset_control_assert(qphy->phy_reset);
>> + if (ret) {
>> + dev_err(&phy->dev, "Failed to assert phy_reset\n");
>> + return ret;
>> + }
>> + /* 100 us delay to keep PHY in reset mode */
>> + usleep_range(100, 150);
>> + ret = reset_control_deassert(qphy->phy_reset);
>> + if (ret) {
>> + dev_err(&phy->dev, "Failed to de-assert phy_reset\n");
>> + return ret;
>> + }
>> +
>> + /* Disable the PHY */
>> + qusb2_setbits(qphy->base + QUSB2PHY_PORT_POWERDOWN,
>> + CLAMP_N_EN | FREEZIO_N | POWER_DOWN);
>> +
>> + /* save reset value to override based on clk scheme */
>> + reset_val = readl_relaxed(qphy->base + QUSB2PHY_PLL_TEST);
>> +
>> + qcom_qusb2_phy_configure(qphy->base, qphy->cfg->phy_init_tbl,
>> + qphy->cfg->phy_init_tbl_sz);
>> +
>> + /* Check for efuse value for tuning the PHY */
>> + ret = qusb2_phy_set_tune2_param(qphy);
>> + if (ret)
>> + return ret;
>> +
>> + /* Enable the PHY */
>> + qusb2_clrbits(qphy->base + QUSB2PHY_PORT_POWERDOWN, POWER_DOWN);
>> +
>> + /* Require to get phy pll lock successfully */
>> + usleep_range(150, 160);
>> +
>> + /* Default is Single-ended clock on msm8996 */
>> + qphy->has_se_clk_scheme = true;
>> + /*
>> + * read TCSR_PHY_CLK_SCHEME register to check if Single-ended
>
> Capital Single again?
will use lowercase.
>
>> + * clock scheme is selected. If yes, then disable differential
>> + * ref_clk and use single-ended clock, otherwise use differential
>> + * ref_clk only.
>> + */
>> + if (qphy->tcsr) {
>> + ret = regmap_read(qphy->tcsr, qphy->cfg->clk_scheme_offset,
>> + &clk_scheme);
>> + /* is it a differential clock scheme ? */
>> + if (!(clk_scheme & PHY_CLK_SCHEME_SEL)) {
>> + dev_vdbg(&phy->dev, "%s: select differential clk src\n",
>> + __func__);
>> + qphy->has_se_clk_scheme = false;
>> + } else {
>> + dev_vdbg(&phy->dev, "%s: select single-ended clk src\n",
>> + __func__);
>> + }
>> + }
>> +
>> + if (!qphy->has_se_clk_scheme) {
>> + reset_val &= ~CLK_REF_SEL;
>> + clk_prepare_enable(qphy->ref_clk);
>
> And if that fails?
will add the check.
>
>> + } else {
>> + reset_val |= CLK_REF_SEL;
>> + }
>> +
>> + writel_relaxed(reset_val, qphy->base + QUSB2PHY_PLL_TEST);
>> +
>> + /* Make sure that above write is completed to get PLL source clock */
>> + wmb();
>> +
>> + /* Required to get PHY PLL lock successfully */
>> + usleep_range(100, 110);
>> +
>> + if (!(readb_relaxed(qphy->base + QUSB2PHY_PLL_STATUS) &
>> + PLL_LOCKED)) {
>> + dev_err(&phy->dev, "QUSB PHY PLL LOCK fails:%x\n",
>> + readb_relaxed(qphy->base + QUSB2PHY_PLL_STATUS));
>
> Would be pretty funny if this was locked now when the error
> printk runs. Are there other bits in there that are helpful?
This is the only bit that's there to check the PLL locking status.
Should we rather poll ?
>
>> + return -EBUSY;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int qusb2_phy_exit(struct phy *phy)
>> +{
>> + struct qusb2_phy *qphy = phy_get_drvdata(phy);
>> +
>> + /* Disable the PHY */
>> + qusb2_setbits(qphy->base + QUSB2PHY_PORT_POWERDOWN,
>> + CLAMP_N_EN | FREEZIO_N | POWER_DOWN);
>> +
>> + if (!qphy->has_se_clk_scheme)
>> + clk_disable_unprepare(qphy->ref_clk);
>> +
>> + clk_disable_unprepare(qphy->cfg_ahb_clk);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct phy_ops qusb2_phy_gen_ops = {
>> + .init = qusb2_phy_init,
>> + .exit = qusb2_phy_exit,
>> + .power_on = qusb2_phy_poweron,
>> + .power_off = qusb2_phy_poweroff,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static const struct of_device_id qusb2_phy_of_match_table[] = {
>> + {
>> + .compatible = "qcom,msm8996-qusb2-phy",
>> + .data = &msm8996_phy_init_cfg,
>> + },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, qusb2_phy_of_match_table);
>> +
>> +static int qusb2_phy_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct qusb2_phy *qphy;
>> + struct phy_provider *phy_provider;
>> + struct phy *generic_phy;
>> + const struct of_device_id *match;
>> + struct resource *res;
>> + int ret;
>> +
>> + qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
>> + if (!qphy)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + qphy->base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(qphy->base))
>> + return PTR_ERR(qphy->base);
>> +
>> + qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb_clk");
>> + if (IS_ERR(qphy->cfg_ahb_clk)) {
>> + ret = PTR_ERR(qphy->cfg_ahb_clk);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "failed to get cfg_ahb_clk\n");
>> + return ret;
>> + }
>> +
>> + qphy->ref_clk_src = devm_clk_get(dev, "ref_clk_src");
>> + if (IS_ERR(qphy->ref_clk_src)) {
>> + ret = PTR_ERR(qphy->ref_clk_src);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "clk get failed for ref_clk_src\n");
>> + return ret;
>> + }
>> +
>> + qphy->ref_clk = devm_clk_get(dev, "ref_clk");
>> + if (IS_ERR(qphy->ref_clk)) {
>> + ret = PTR_ERR(qphy->ref_clk);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "clk get failed for ref_clk\n");
>> + return ret;
>> + } else {
>> + clk_set_rate(qphy->ref_clk, 19200000);
>
> Drop the else. Also what if clk_set_rate() fails?
Will drop the else.
we should fail in case clk_set_rate() fails.
>
>> + }
>> +
>> + qphy->iface_clk = devm_clk_get(dev, "iface_clk");
>> + if (IS_ERR(qphy->iface_clk)) {
>> + ret = PTR_ERR(qphy->iface_clk);
>> + if (ret != -EPROBE_DEFER) {
>> + qphy->iface_clk = NULL;
>> + dev_dbg(dev, "clk get failed for iface_clk\n");
>> + } else {
>> + return ret;
>> + }
>
> if (PTR_ERR(qphy->iface_clk) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
> qphy->iface_clk = NULL;
> dev_dbg(dev, "clk get failed for iface_clk\n");
>
> Is shorter.
Sure, will modify this as suggested.
>
>> + }
>> +
>> + qphy->phy_reset = devm_reset_control_get(&pdev->dev, "phy");
>> + if (IS_ERR(qphy->phy_reset)) {
>> + dev_err(dev, "failed to get phy core reset\n");
>> + return PTR_ERR(qphy->phy_reset);
>> + }
>> +
>> + qphy->vdd_phy = devm_regulator_get(dev, "vdd-phy");
>> + if (IS_ERR(qphy->vdd_phy)) {
>> + dev_err(dev, "unable to get vdd-phy supply\n");
>> + return PTR_ERR(qphy->vdd_phy);
>> + }
>> +
>> + qphy->vdda_pll = devm_regulator_get(dev, "vdda-pll");
>> + if (IS_ERR(qphy->vdda_pll)) {
>> + dev_err(dev, "unable to get vdda-pll supply\n");
>> + return PTR_ERR(qphy->vdda_pll);
>> + }
>> +
>> + qphy->vdda_phy_dpdm = devm_regulator_get(dev, "vdda-phy-dpdm");
>> + if (IS_ERR(qphy->vdda_phy_dpdm)) {
>> + dev_err(dev, "unable to get vdda-phy-dpdm supply\n");
>> + return PTR_ERR(qphy->vdda_phy_dpdm);
>> + }
>> +
>> + /* Get the specific init parameters of QMP phy */
>> + match = of_match_node(qusb2_phy_of_match_table, dev->of_node);
>> + qphy->cfg = match->data;
>
> Use of_device_get_match_data() instead.
Okay.
>
>> +
>> + qphy->tcsr = syscon_regmap_lookup_by_phandle(dev->of_node,
>> + "qcom,tcsr-syscon");
>> + if (IS_ERR(qphy->tcsr)) {
>> + dev_dbg(dev, "Failed to lookup TCSR regmap\n");
>> + qphy->tcsr = NULL;
>> + }
>> +
>> + generic_phy = devm_phy_create(dev, NULL, &qusb2_phy_gen_ops);
>> + if (IS_ERR(generic_phy)) {
>> + ret = PTR_ERR(generic_phy);
>> + dev_err(dev, "%s: failed to create phy %d\n", __func__, ret);
>> + return ret;
>> + }
>> + qphy->phy = generic_phy;
>> +
>> + dev_set_drvdata(dev, qphy);
>> + phy_set_drvdata(generic_phy, qphy);
>> +
>> + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> + if (IS_ERR(phy_provider)) {
>> + ret = PTR_ERR(phy_provider);
>> + dev_err(dev, "%s: failed to register phy %d\n", __func__, ret);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
Thanks for a thorough review. Will respin the patch soon.
regards
Vivek
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 3/7] PWM: add pwm-stm32 DT bindings
From: Benjamin Gaignard @ 2016-12-01 8:44 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: <20161130212055.b57qkqy66j5gg57q@rob-hp-laptop>
2016-11-30 22:20 GMT+01:00 Rob Herring <robh@kernel.org>:
> On Thu, Nov 24, 2016 at 04:14:19PM +0100, Benjamin Gaignard wrote:
>> Define bindings for pwm-stm32
>>
>> version 2:
>> - use parameters instead of compatible of handle the hardware configuration
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>> .../devicetree/bindings/pwm/pwm-stm32.txt | 37 ++++++++++++++++++++++
>> 1 file changed, 37 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-stm32.txt b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> new file mode 100644
>> index 0000000..36263f0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-stm32.txt
>> @@ -0,0 +1,37 @@
>> +STMicroelectronics PWM driver bindings for STM32
>> +
>> +Must be a sub-node of STM32 general purpose timer driver
>> +
>> +Required parameters:
>> +- compatible: Must be "st,stm32-pwm"
>> +- pinctrl-names: Set to "default".
>> +- pinctrl-0: List of phandles pointing to pin configuration nodes
>> + for PWM module.
>> + For Pinctrl properties, please refer to [1].
>> +
>> +Optional parameters:
>> +- st,breakinput: Set if the hardware have break input capabilities
>> +- st,breakinput-polarity: Set break input polarity. Default is 0
>> + The value define the active polarity:
>> + - 0 (active LOW)
>> + - 1 (active HIGH)
>> +- st,pwm-num-chan: Number of available PWM channels. Default is 0.
>> +- st,32bits-counter: Set if the hardware have a 32 bits counter
>> +- st,complementary: Set if the hardware have complementary output channels
>
> What does complementary mean here?
Complementary channels are pwm channels where the signal level is inverted
compare to the original channel.
This parameter indicate that the hardware have this kind of outputs.
If the polarity of the original channel change then polarity of
complementary channel
change too.
>
>> +
>> +[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>> +
>> +Example:
>> + gptimer1: gptimer1@40010000 {
>
> timer@...
I would like keep "timer" for timer-trigger sub-node
>
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40010000 0x400>;
>> + clocks = <&rcc 0 160>;
>> + clock-names = "clk_int";
>> +
>> + pwm1@0 {
>
> pwm {
>
> Is there more than one?
Not per hardware block but their is 12 of them in the SoC.
Adding a number (which match with SoC documentation) help to find
the wanted pwm in sysfs either we only have the address.
>
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,breakinput;
>> + st,complementary;
>> + };
>> + };
>> --
>> 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: Re: [RFC PATCH] ARM: dts: sun8i: add simplefb node for H3
From: Maxime Ripard @ 2016-12-01 8:45 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Jernej Škrabec, Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
wens-jdAy2FN1RRM@public.gmane.org, linux-kernel, linux-sunxi,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1777251480557779-y7RzsOdW5YFxpj1cXAZ9Bg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]
On Thu, Dec 01, 2016 at 10:02:59AM +0800, Icenowy Zheng wrote:
>
>
> 01.12.2016, 04:52, "Maxime Ripard" <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>:
> > On Wed, Nov 30, 2016 at 09:41:26PM +0100, Jernej Škrabec wrote:
> >> > > > > The only
> >> > > > > code left from you is for DE2. HDMI stuff is basically copied from
> >> > > > > Rockhip
> >> > > > > driver (including EDID reading), TCON code is now reverted to the same
> >> > > > > as
> >> > > > > it is in sunxi_display.c. I think it is worth to take a look at EDID
> >> > > > > code
> >> > > > > and compare it.
> >> > > >
> >> > > > So is the TCON of DE 2.0 identical to the original TCON?
> >> > > >
> >> > > > If so, we should reuse sun4i-tcon ...
> >> > >
> >> > > Well, TCON is splitted in two parts (two base addresses), one for HDMI and
> >> > > one for TV. However, register offsets are same as before, so I guess
> >> > > driver reusage make sense. I think that there are few additional
> >> > > registers, but they can be ignored for simplefb.
> >> >
> >> > The TCON1 of the H3 is not usable (no ckock). Analog TV has its own
> >> > clock and I/O area.
> >> >
> >>
> >> True, H3 user manual can be misleading sometimes. But this doesn't change the
> >> fact that TCON0 has same register offsets with same meaning.
> >
> > Then yes, we should definitely share the drivers too. So, in the end,
> > the only thing that is actually new is the display-engine?
>
> And HDMI PHY on H3 ;-)
Yes, and that one :)
> In my opinion, we should just put sun8i-de2-drm related code into
> drivers/gpu/drm/sun4i/ . (Or rename the directory to sunxi)
We should definitely reuse the drivers that are already in there for
the TCON and HDMI parts.
I'd have to look at the exact amount of code that would be needed to
support the new display engine, but I guess if it's big, then a
separate folder makes sense, if it isn't, then putting it in sun4i
makes sense (but I'm reluctant to renaming it to sunxi).
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] Synopsys USB 2.0 Device Controller (UDC) Driver
From: Felipe Balbi @ 2016-12-01 8:53 UTC (permalink / raw)
To: Raviteja Garimella
Cc: Rob Herring, Mark Rutland, Greg Kroah-Hartman,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, BCM Kernel Feedback,
linux-usb-u79uwXL29TY76Z2rM5mHXA, John Youn
In-Reply-To: <7de6b970-c88c-b277-eaae-d9c0067b590f-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1535 bytes --]
Hi,
John Youn <John.Youn-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> writes:
> On 11/30/2016 4:47 AM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Raviteja Garimella <raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> writes:
>>> Hi Balbi,
>>>
>>> On Wed, Nov 30, 2016 at 4:10 PM, Felipe Balbi <balbi-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>>>
>>>> Hi,
>>>>
>>>> Raviteja Garimella <raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> writes:
>>>>> This is driver for Synopsys Designware Cores USB Device
>>>>> Controller (UDC) Subsystem with the AMBA Advanced High-Performance
>>>>> Bus (AHB). This driver works with Synopsys UDC20 products.
>>>>>
>>>>> Signed-off-by: Raviteja Garimella <raviteja.garimella-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>>
>>>> use drivers/usb/dwc2 instead of duplicating it.
>>>
>>> The ones we have in drivers/usb/dwc2 is for Designware high speed OTG
>>> controller IP. The one that I submitted for review is for USB Device
>>> controller IP (UDC). The IPs are different.
>>
>> I'll wait for John's confirmation that this really isn't compatible with
>> dwc2. John?
>>
>
> Hi Felipe,
>
> This is our older UDC IP, not compatible with HSOTG.
>
> It is also no longer supported by Synopsys and considered EOL.
Is it the same one used by amd5536udc.c? If it is, then it's much better
to refactor that driver so it can be used as a library of sorts by PCI
and non-PCI systems. We really don't want duplicated drivers upstream.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: Re: [PATCH v7 4/8] drm/sunxi: Add DT bindings documentation of Allwinner HDMI
From: Maxime Ripard @ 2016-12-01 8:55 UTC (permalink / raw)
To: Icenowy Zheng
Cc: moinejf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, Laurent Pinchart,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
Dave Airlie, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <1971681480527210-RAIFsGP/6J5uio3avFS2gg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]
On Thu, Dec 01, 2016 at 01:33:30AM +0800, Icenowy Zheng wrote:
> >> hdmi-out {
> >> compatible = "hdmi-connector";
> >> type = "a";
> >> /* I2C bus and GPIO references are made up for the example */
> >> ddc-i2c-bus = <&i2c4>;
> >> hpd-gpios = <&gpio4 7 GPIO_ACTIVE_HIGH>
> >
> > the "hdmi-connector" is a big piece of software. It must handle a lot
> > of more and more exotic connectors.
> > So, I hope that you have written a "simple-hdmi-connector" which does
> > nothing but setting the connector type.
> > Where is it?
>
> I suddenly thought about something...
>
> If a DVI connector instead of a HDMI connector is soldered, how
> should such a device tree be written?
Use a dvi-connector instead :)
> How about solder a HDMI-to-VGA bridge on the board? (Maybe there
> should be "dumb-hdmi-dvi-bridge" and "dumb-hdmi-vga-bridge"
> drivers?)
It probably wouldn't be dumb, but yeah, it would definitely be a
bridge instead of the connector.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v2 4/9] clk: stm32f4: Add lcd-tft clock
From: Gabriel Fernandez @ 2016-12-01 8:58 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, daniel.thompson, radoslaw.pietrzyk,
Alexandre Torgue, Arnd Bergmann, Nicolas Pitre, andrea.merello,
Michael Turquette, olivier.bideau, Stephen Boyd, Russell King,
linux-kernel, ludovic.barre, Maxime Coquelin, amelie.delaunay,
linux-clk, linux-arm-kernel, kernel
In-Reply-To: <20161130205333.tiukcjbspuqmedgw@rob-hp-laptop>
Hi Rob,
Thanks for reviewing
On 11/30/2016 09:53 PM, Rob Herring wrote:
> On Thu, Nov 24, 2016 at 03:45:44PM +0100, gabriel.fernandez@st.com wrote:
>> From: Gabriel Fernandez <gabriel.fernandez@st.com>
>>
>> This patch introduces lcd-tft clock for stm32f4 soc.
>>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>> ---
>> .../devicetree/bindings/clock/st,stm32-rcc.txt | 1 +
>> drivers/clk/clk-stm32f4.c | 118 +++++++++++++++++++++
>> include/dt-bindings/clock/stm32f4-clock.h | 3 +-
>> 3 files changed, 121 insertions(+), 1 deletion(-)
>
>> diff --git a/include/dt-bindings/clock/stm32f4-clock.h b/include/dt-bindings/clock/stm32f4-clock.h
>> index 56b8e10..1be4a3a 100644
>> --- a/include/dt-bindings/clock/stm32f4-clock.h
>> +++ b/include/dt-bindings/clock/stm32f4-clock.h
>> @@ -27,7 +27,8 @@
>> #define CLK_RTC 5
>> #define PLL_VCO_I2S 6
>> #define PLL_VCO_SAI 7
>> +#define CLK_LCD 8
>>
>> -#define END_PRIMARY_CLK 8
>> +#define END_PRIMARY_CLK 9
> Do you really need this? Having this change could cause compatibility
> problems between dtb and kernel versions.
>
> Please restructure the patch series and put all of the binding changes
> including this header into a single patch. Incrementally add s/w
> features, not h/w.
>
> Rob
Okay
Best Regards
Gabriel
^ permalink raw reply
* [PATCH v4 2/9] doc: DT: venus: binding document for Qualcomm video driver
From: Stanimir Varbanov @ 2016-12-01 9:03 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Hans Verkuil
Cc: Andy Gross, Bjorn Andersson, Stephen Boyd, Srinivas Kandagatla,
linux-media, linux-kernel, linux-arm-msm, Stanimir Varbanov,
Rob Herring, Mark Rutland, devicetree
In-Reply-To: <1480583001-32236-1-git-send-email-stanimir.varbanov@linaro.org>
Add binding document for Venus video encoder/decoder driver
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
Rob, I have removed vmem clocks, interrupts and reg properties
for vmem thing. Probably I will come with a separate platform
driver fro that and pass the video memory DT node as phandle.
.../devicetree/bindings/media/qcom,venus.txt | 82 ++++++++++++++++++++++
1 file changed, 82 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/qcom,venus.txt
diff --git a/Documentation/devicetree/bindings/media/qcom,venus.txt b/Documentation/devicetree/bindings/media/qcom,venus.txt
new file mode 100644
index 000000000000..a64b4ea1ebba
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/qcom,venus.txt
@@ -0,0 +1,82 @@
+* Qualcomm Venus video encode/decode accelerator
+
+- compatible:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Value should contain one of:
+ - "qcom,msm8916-venus"
+ - "qcom,msm8996-venus"
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Register ranges as listed in the reg-names property.
+- reg-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain following entries:
+ - "base" Venus register base
+- interrupts:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Should contain interrupts as listed in the interrupt-names
+ property.
+- interrupt-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain following entries:
+ - "venus" Venus interrupt line
+- clocks:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: A List of phandle and clock specifier pairs as listed
+ in clock-names property.
+- clock-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain the following entries:
+ - "core" Core video accelerator clock
+ - "iface" Video accelerator AHB clock
+ - "bus" Video accelerator AXI clock
+- clock-names:
+ Usage: required for msm8996
+ Value type: <stringlist>
+ Definition: Should contain the following entries:
+ - "subcore0" Subcore0 video accelerator clock
+ - "subcore1" Subcore1 video accelerator clock
+ - "mmssnoc_axi" Multimedia subsystem NOC AXI clock
+ - "mmss_mmagic_iface" Multimedia subsystem MMAGIC AHB clock
+ - "mmss_mmagic_mbus" Multimedia subsystem MMAGIC MAXI clock
+ - "mmagic_video_bus" MMAGIC video AXI clock
+ - "video_mbus" Video MAXI clock
+- power-domains:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: A phandle and power domain specifier pairs to the
+ power domain which is responsible for collapsing
+ and restoring power to the peripheral.
+- rproc:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: A phandle to remote processor responsible for
+ firmware loading and processor booting.
+
+- iommus:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: A list of phandle and IOMMU specifier pairs.
+
+* An Example
+ video-codec@1d00000 {
+ compatible = "qcom,msm8916-venus";
+ reg = <0x01d00000 0xff000>;
+ reg-names = "base";
+ interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "venus";
+ clocks = <&gcc GCC_VENUS0_VCODEC0_CLK>,
+ <&gcc GCC_VENUS0_AHB_CLK>,
+ <&gcc GCC_VENUS0_AXI_CLK>;
+ clock-names = "core", "iface", "bus";
+ power-domains = <&gcc VENUS_GDSC>;
+ rproc = <&venus_rproc>;
+ iommus = <&apps_iommu 5>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v3 0/4] drm: Add support for the Amlogic Video Processing Unit
From: Neil Armstrong @ 2016-12-01 9:05 UTC (permalink / raw)
To: airlied, khilman, carlo
Cc: Neil Armstrong, dri-devel, linux-amlogic, linux-arm-kernel,
linux-kernel, victor.wan, jerry.cao, Xing.Xu, laurent.pinchart,
daniel, devicetree
This a repost of the previous version at [3] with fixes, the following patches will
be sent via a PULL Request once the DT maintainers acks the DT bindings.
The Amlogic maintainer will take the arm64 DT patches to avoid merges conflicts.
The Amlogic Meson SoCs embeds a Video Processing Unit able to output at least
a Composite/CVBS Video with embedded VDAC and an HDMI Link with Embedded HDMI
Transceiver.
Thus, the current driver does not support HDMI yet.
The Video Processig Unit is composed of multiple modules like the Video
Input Unit and the Video Post Processing that can be associated to a
CRTC with Planes management.
The last Unit is the Venc that embeds at least 3 Encoders, ENCI for Interlace
Video used by CVBS or HDMI, ENCP for Progressive Video used by the HDMI
Transceiver and ENCL for LCD Display.
The LCD Display is not planned to be supported on the Meson GX Family.
This driver is a DRM/KMS driver using the following DRM components :
- GEM-CMA
- PRIME-CMA
- Atomic Modesetting
- FBDev-CMA
For the following SoCs :
- GXBB Family (S905)
- GXL Family (S905X, S905D)
- GXM Family (S912)
The current driver only supports the CVBS PAL/NTSC output modes, but the
CRTC/Planes management should support bigger modes.
But Advanced Colorspace Conversion, Scaling and HDMI Modes will be added in
a second time.
The Device Tree bindings makes use of the endpoints video interface definitions
to connect to the optional CVBS and in the future the HDMI Connector nodes.
The driver has been tested with Xorg modesetting driver and Weston DRM backend.
Changes since v1 at [3] :
- Rename and move dt-bindings file
- Clarified VPU node graph outputs
- Removed all of_machine_is_compatible
- Cleaned DTS endpoint and board connectors nodes
- Added Laurent Pinchard's ack and reviewed-bys
Changes since v1 at [2] :
- Simplify bindings to have a "composite-video-connector" to represent the on-board composite connector
- Remove the component_match binding since it's unused for now
- Moved all DRM connector code back in the venc_cvbs file
- Check for port endpoints validity to detech connector existence
- Added Daniel Vetter's ack on non-dt patches commit messages
Changes since RFC at [1] :
- Add maintainers entry
- Move all Plane and CRTC code from backend to corresponding DRM code
- Keep only init and common code in backend source files
- Move the CVBS encoder out of the CVBS DT node, only keep the connector
- Various cleanups using DRM helpers
- Cleanup of copyright headers
- Fixup of bindings documentation
[1] http://lkml.kernel.org/r/1480089791-12517-1-git-send-email-narmstrong@baylibre.com
[2] http://lkml.kernel.org/r/1480416469-9655-1-git-send-email-narmstrong@baylibre.com
[3] http://lkml.kernel.org/r/1480520625-13269-1-git-send-email-narmstrong@baylibre.com
Neil Armstrong (4):
drm: Add support for Amlogic Meson Graphic Controller
ARM64: dts: meson-gx: Add Graphic Controller nodes
dt-bindings: display: add Amlogic Meson DRM Bindings
MAINTAINERS: add entry for Amlogic DRM drivers
.../bindings/display/amlogic,meson-vpu.txt | 112 ++
MAINTAINERS | 9 +
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 16 +
.../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 16 +
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 16 +
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 +
.../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 16 +
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 4 +
.../arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 16 +
arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 4 +
drivers/gpu/drm/Kconfig | 2 +
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/meson/Kconfig | 9 +
drivers/gpu/drm/meson/Makefile | 4 +
drivers/gpu/drm/meson/meson_canvas.c | 68 +
drivers/gpu/drm/meson/meson_canvas.h | 42 +
drivers/gpu/drm/meson/meson_crtc.c | 208 +++
drivers/gpu/drm/meson/meson_crtc.h | 32 +
drivers/gpu/drm/meson/meson_drv.c | 343 +++++
drivers/gpu/drm/meson/meson_drv.h | 59 +
drivers/gpu/drm/meson/meson_plane.c | 230 ++++
drivers/gpu/drm/meson/meson_plane.h | 30 +
drivers/gpu/drm/meson/meson_registers.h | 1395 ++++++++++++++++++++
drivers/gpu/drm/meson/meson_vclk.c | 167 +++
drivers/gpu/drm/meson/meson_vclk.h | 34 +
drivers/gpu/drm/meson/meson_venc.c | 254 ++++
drivers/gpu/drm/meson/meson_venc.h | 72 +
drivers/gpu/drm/meson/meson_venc_cvbs.c | 293 ++++
drivers/gpu/drm/meson/meson_venc_cvbs.h | 41 +
drivers/gpu/drm/meson/meson_viu.c | 331 +++++
drivers/gpu/drm/meson/meson_viu.h | 64 +
drivers/gpu/drm/meson/meson_vpp.c | 162 +++
drivers/gpu/drm/meson/meson_vpp.h | 35 +
33 files changed, 4089 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
create mode 100644 drivers/gpu/drm/meson/Kconfig
create mode 100644 drivers/gpu/drm/meson/Makefile
create mode 100644 drivers/gpu/drm/meson/meson_canvas.c
create mode 100644 drivers/gpu/drm/meson/meson_canvas.h
create mode 100644 drivers/gpu/drm/meson/meson_crtc.c
create mode 100644 drivers/gpu/drm/meson/meson_crtc.h
create mode 100644 drivers/gpu/drm/meson/meson_drv.c
create mode 100644 drivers/gpu/drm/meson/meson_drv.h
create mode 100644 drivers/gpu/drm/meson/meson_plane.c
create mode 100644 drivers/gpu/drm/meson/meson_plane.h
create mode 100644 drivers/gpu/drm/meson/meson_registers.h
create mode 100644 drivers/gpu/drm/meson/meson_vclk.c
create mode 100644 drivers/gpu/drm/meson/meson_vclk.h
create mode 100644 drivers/gpu/drm/meson/meson_venc.c
create mode 100644 drivers/gpu/drm/meson/meson_venc.h
create mode 100644 drivers/gpu/drm/meson/meson_venc_cvbs.c
create mode 100644 drivers/gpu/drm/meson/meson_venc_cvbs.h
create mode 100644 drivers/gpu/drm/meson/meson_viu.c
create mode 100644 drivers/gpu/drm/meson/meson_viu.h
create mode 100644 drivers/gpu/drm/meson/meson_vpp.c
create mode 100644 drivers/gpu/drm/meson/meson_vpp.h
--
1.9.1
^ permalink raw reply
* [PATCH v3 2/4] ARM64: dts: meson-gx: Add Graphic Controller nodes
From: Neil Armstrong @ 2016-12-01 9:05 UTC (permalink / raw)
To: airlied, khilman, carlo
Cc: Neil Armstrong, dri-devel, linux-amlogic, linux-arm-kernel,
linux-kernel, victor.wan, jerry.cao, Xing.Xu, devicetree,
laurent.pinchart, daniel
In-Reply-To: <1480583160-31806-1-git-send-email-narmstrong@baylibre.com>
Add Video Processing Unit and CVBS Output nodes, and enable CVBS on selected
boards.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 16 ++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 16 ++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 16 ++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 4 ++++
arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 16 ++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 4 ++++
arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 16 ++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxm.dtsi | 4 ++++
8 files changed, 92 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index fc033c0..eada0b5 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -356,5 +356,21 @@
status = "disabled";
};
};
+
+ vpu: vpu@d0100000 {
+ compatible = "amlogic,meson-gx-vpu";
+ reg = <0x0 0xd0100000 0x0 0x100000>,
+ <0x0 0xc883c000 0x0 0x1000>,
+ <0x0 0xc8838000 0x0 0x1000>;
+ reg-names = "vpu", "hhi", "dmc";
+ interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* CVBS VDAC output port */
+ cvbs_vdac_port: port@0 {
+ reg = <0>;
+ };
+ };
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
index 9696820..4cbd626 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
@@ -142,6 +142,16 @@
clocks = <&wifi32k>;
clock-names = "ext_clock";
};
+
+ cvbs-connector {
+ compatible = "composite-video-connector";
+
+ port {
+ cvbs_connector_in: endpoint {
+ remote-endpoint = <&cvbs_vdac_out>;
+ };
+ };
+ };
};
&uart_AO {
@@ -229,3 +239,9 @@
clocks = <&clkc CLKID_FCLK_DIV4>;
clock-names = "clkin0";
};
+
+&cvbs_vdac_port {
+ cvbs_vdac_out: endpoint {
+ remote-endpoint = <&cvbs_connector_in>;
+ };
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 203be28..4a96e0f 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -125,6 +125,16 @@
clocks = <&wifi32k>;
clock-names = "ext_clock";
};
+
+ cvbs-connector {
+ compatible = "composite-video-connector";
+
+ port {
+ cvbs_connector_in: endpoint {
+ remote-endpoint = <&cvbs_vdac_out>;
+ };
+ };
+ };
};
/* This UART is brought out to the DB9 connector */
@@ -234,3 +244,9 @@
clocks = <&clkc CLKID_FCLK_DIV4>;
clock-names = "clkin0";
};
+
+&cvbs_vdac_port {
+ cvbs_vdac_out: endpoint {
+ remote-endpoint = <&cvbs_connector_in>;
+ };
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index ac5ad3b..5353a20 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -506,3 +506,7 @@
<&clkc CLKID_FCLK_DIV2>;
clock-names = "core", "clkin0", "clkin1";
};
+
+&vpu {
+ compatible = "amlogic,meson-gxbb-vpu", "amlogic,meson-gx-vpu";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
index e99101a..cea4a3e 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
@@ -117,6 +117,16 @@
clocks = <&wifi32k>;
clock-names = "ext_clock";
};
+
+ cvbs-connector {
+ compatible = "composite-video-connector";
+
+ port {
+ cvbs_connector_in: endpoint {
+ remote-endpoint = <&cvbs_vdac_out>;
+ };
+ };
+ };
};
&uart_AO {
@@ -203,3 +213,9 @@
clocks = <&clkc CLKID_FCLK_DIV4>;
clock-names = "clkin0";
};
+
+&cvbs_vdac_port {
+ cvbs_vdac_out: endpoint {
+ remote-endpoint = <&cvbs_connector_in>;
+ };
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
index 9f89b99..5c7a8fa 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
@@ -299,3 +299,7 @@
<&clkc CLKID_FCLK_DIV2>;
clock-names = "core", "clkin0", "clkin1";
};
+
+&vpu {
+ compatible = "amlogic,meson-gxl-vpu", "amlogic,meson-gx-vpu";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index d320727..f2d0861 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -90,6 +90,16 @@
compatible = "mmc-pwrseq-emmc";
reset-gpios = <&gpio BOOT_9 GPIO_ACTIVE_LOW>;
};
+
+ cvbs-connector {
+ compatible = "composite-video-connector";
+
+ port {
+ cvbs_connector_in: endpoint {
+ remote-endpoint = <&cvbs_vdac_out>;
+ };
+ };
+ };
};
/* This UART is brought out to the DB9 connector */
@@ -167,3 +177,9 @@
max-speed = <1000>;
};
};
+
+&cvbs_vdac_port {
+ cvbs_vdac_out: endpoint {
+ remote-endpoint = <&cvbs_connector_in>;
+ };
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
index c1974bb..eb2f0c3 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm.dtsi
@@ -112,3 +112,7 @@
};
};
};
+
+&vpu {
+ compatible = "amlogic,meson-gxm-vpu", "amlogic,meson-gx-vpu";
+};
--
1.9.1
^ permalink raw reply related
* [PATCH v3 3/4] dt-bindings: display: add Amlogic Meson DRM Bindings
From: Neil Armstrong @ 2016-12-01 9:05 UTC (permalink / raw)
To: airlied, khilman, carlo
Cc: devicetree, Xing.Xu, victor.wan, Neil Armstrong, linux-kernel,
dri-devel, laurent.pinchart, daniel, jerry.cao, linux-amlogic,
linux-arm-kernel
In-Reply-To: <1480583160-31806-1-git-send-email-narmstrong@baylibre.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../bindings/display/amlogic,meson-vpu.txt | 112 +++++++++++++++++++++
1 file changed, 112 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
new file mode 100644
index 0000000..00f74ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.txt
@@ -0,0 +1,112 @@
+Amlogic Meson Display Controller
+================================
+
+The Amlogic Meson Display controller is composed of several components
+that are going to be documented below:
+
+DMC|---------------VPU (Video Processing Unit)----------------|------HHI------|
+ | vd1 _______ _____________ _________________ | |
+D |-------| |----| | | | | HDMI PLL |
+D | vd2 | VIU | | Video Post | | Video Encoders |<---|-----VCLK |
+R |-------| |----| Processing | | | | |
+ | osd2 | | | |---| Enci ----------|----|-----VDAC------|
+R |-------| CSC |----| Scalers | | Encp ----------|----|----HDMI-TX----|
+A | osd1 | | | Blenders | | Encl ----------|----|---------------|
+M |-------|______|----|____________| |________________| | |
+___|__________________________________________________________|_______________|
+
+
+VIU: Video Input Unit
+---------------------
+
+The Video Input Unit is in charge of the pixel scanout from the DDR memory.
+It fetches the frames addresses, stride and parameters from the "Canvas" memory.
+This part is also in charge of the CSC (Colorspace Conversion).
+It can handle 2 OSD Planes and 2 Video Planes.
+
+VPP: Video Post Processing
+--------------------------
+
+The Video Post Processing is in charge of the scaling and blending of the
+various planes into a single pixel stream.
+There is a special "pre-blending" used by the video planes with a dedicated
+scaler and a "post-blending" to merge with the OSD Planes.
+The OSD planes also have a dedicated scaler for one of the OSD.
+
+VENC: Video Encoders
+--------------------
+
+The VENC is composed of the multiple pixel encoders :
+ - ENCI : Interlace Video encoder for CVBS and Interlace HDMI
+ - ENCP : Progressive Video Encoder for HDMI
+ - ENCL : LCD LVDS Encoder
+The VENC Unit gets a Pixel Clocks (VCLK) from a dedicated HDMI PLL and clock
+tree and provides the scanout clock to the VPP and VIU.
+The ENCI is connected to a single VDAC for Composite Output.
+The ENCI and ENCP are connected to an on-chip HDMI Transceiver.
+
+Device Tree Bindings:
+---------------------
+
+VPU: Video Processing Unit
+--------------------------
+
+Required properties:
+- compatible: value should be different for each SoC family as :
+ - GXBB (S905) : "amlogic,meson-gxbb-vpu"
+ - GXL (S905X, S905D) : "amlogic,meson-gxl-vpu"
+ - GXM (S912) : "amlogic,meson-gxm-vpu"
+ followed by the common "amlogic,meson-gx-vpu"
+- reg: base address and size of he following memory-mapped regions :
+ - vpu
+ - hhi
+ - dmc
+- reg-names: should contain the names of the previous memory regions
+- interrupts: should contain the VENC Vsync interrupt number
+
+Required nodes:
+
+The connections to the VPU output video ports are modeled using the OF graph
+bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+The following table lists for each supported model the port number
+corresponding to each VPU output.
+
+ Port 0 Port 1
+-----------------------------------------
+ S905 (GXBB) CVBS VDAC HDMI-TX
+ S905X (GXL) CVBS VDAC HDMI-TX
+ S905D (GXL) CVBS VDAC HDMI-TX
+ S912 (GXM) CVBS VDAC HDMI-TX
+
+Example:
+
+tv-connector {
+ compatible = "composite-video-connector";
+
+ port {
+ tv_connector_in: endpoint {
+ remote-endpoint = <&cvbs_vdac_out>;
+ };
+ };
+};
+
+vpu: vpu@d0100000 {
+ compatible = "amlogic,meson-gxbb-vpu";
+ reg = <0x0 0xd0100000 0x0 0x100000>,
+ <0x0 0xc883c000 0x0 0x1000>,
+ <0x0 0xc8838000 0x0 0x1000>;
+ reg-names = "vpu", "hhi", "dmc";
+ interrupts = <GIC_SPI 3 IRQ_TYPE_EDGE_RISING>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* CVBS VDAC output port */
+ port@0 {
+ reg = <0>;
+
+ cvbs_vdac_out: endpoint {
+ remote-endpoint = <&tv_connector_in>;
+ };
+ };
+};
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 2/2] dt-bindings: Add DT bindings info for FlexRM mailbox driver
From: Anup Patel @ 2016-12-01 9:11 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: <20161130213854.jmjwcis2zid6busv@rob-hp-laptop>
On Thu, Dec 1, 2016 at 3:08 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Nov 25, 2016 at 10:05:51AM +0530, Anup Patel wrote:
>> This patch adds device tree bindings document for the FlexRM
>> mailbox driver.
>
> Bindings document h/w, not drivers.
OK, I will rephrase this.
>
>>
>> Reviewed-by: Ray Jui <ray.jui-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>> .../bindings/mailbox/brcm,flexrm-mbox.txt | 60 ++++++++++++++++++++++
>> 1 file changed, 60 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
>>
>> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
>> new file mode 100644
>> index 0000000..7969b1c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mailbox/brcm,flexrm-mbox.txt
>> @@ -0,0 +1,60 @@
>> +Broadcom FlexRM Mailbox Driver
>
> h/w
OK, I will rephrase this.
>
>> +===============================
>> +The Broadcom FlexRM ring manager provides a set of rings which can be
>> +used to submit work to offload engines. An SoC may have multiple FlexRM
>> +hardware blocks. There is one device tree entry per block. The FlexRM
>> +mailbox drivers creates a mailbox instance using FlexRM rings where
>> +each mailbox channel is a separate FlexRM ring.
>> +
>> +Required properties:
>> +--------------------
>> +- compatible: Should be "brcm,flexrm-mbox"
>
> Sounds generic. Add SoC specific compatible strings please.
OK, will do.
>
>> +- reg: Specifies base physical address and size of the FlexRM
>> + ring registers
>> +- msi-parent: Phandles (and potential Device IDs) to MSI controllers
>> + The FlexRM engine will send MSIs (instead of wired
>> + interrupts) to CPU. There is one MSI for each FlexRM ring.
>
> One ring is one h/w block, right? How many instances is not really
> relevant.
No, FlexRM is the HW block. One instance of FlexRM provides a set of
rings (typically 32 or more).
There are lot other registers in FlexRM apart from ring registers. Out of
these, only ring registers are accessible to non-secured world (i.e. Linux)
whereas all other registers are only accessible to secure-world firmware
(typically ATF).
>
>> + Refer devicetree/bindings/interrupt-controller/msi.txt
>> +- #mbox-cells: Specifies the number of cells needed to encode a mailbox
>> + channel. This should be 3.
>> +
>> + The 1st cell is the mailbox channel number.
>> +
>> + The 2nd cell contains MSI completion threshold. This is the
>> + number of completion messages for which FlexRM will inject
>> + one MSI interrupt to CPU.
>> +
>> + The 3nd cell contains MSI timer value representing time for
>> + which FlexRM will wait to accumulate N completion messages
>> + where N is the value specified by 2nd cell above. If FlexRM
>> + does not get required number of completion messages in time
>> + specified by this cell then it will inject one MSI interrupt
>> + to CPU provided atleast one completion message is available.
>> +
>> +Optional properties:
>> +--------------------
>> +- dma-coherent: Present if DMA operations made by the FlexRM engine (such
>> + as DMA descriptor access, access to buffers pointed by DMA
>> + descriptors and read/write pointer updates to DDR) are
>> + cache coherent with the CPU.
>> +
>> +Example:
>> +--------
>> +crypto_mbox: mbox@67000000 {
>> + compatible = "brcm,flexrm-mbox";
>> + reg = <0x67000000 0x200000>;
>> + msi-parent = <&gic_its 0x7f00>;
>> + #mbox-cells = <3>;
>> +};
>> +
>> +crypto_client {
>
> Is this a h/w block or purely a driver on top of the mailbox? The latter
> doesn't belong in DT.
The FlexRM is one HW block. It provides ring-based programming
interface to various offload engines which are separate HW blocks.
The driver for FlexRM is implemented as mailbox controller driver
while the offload engine drivers will be mailbox clients.
>
>> + ...
>> + 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>;
>> + };
>> + ...
>> +};
Regards,
Anup
--
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 v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Maxime Ripard @ 2016-12-01 9:13 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <2039137.LtCOH6bs2I@avalon>
[-- Attachment #1: Type: text/plain, Size: 3730 bytes --]
Hi Laurent,
On Wed, Nov 30, 2016 at 12:12:55PM +0200, Laurent Pinchart wrote:
> > More, it is not sure that the bridge/DW code would work with Allwinner's
> > SoCs.
>
> If it doesn't work and can't be made to work in a non-invasive way they it
> should certainly not be used :-)
Even if the register layout is completely scrambled, as long as the
bits themselves aren't (and by comparing the two drivers it looks like
they haven't changed), you can easily deal with that using the
regmap_fields, with the two implementations (the original one and the
scrambled one) providing their register map that way, and the driver
code using whatever has been provided.
> > Eventually, I went the same way as omap/hdmi5: different driver.
>
> I might try to fix that for OMAP5 at some point, we'll see.
For complex drivers that have already a driver written and a lot of
testing that already happened, I don't think duplication is a smart
move.
> > > - And finally the fact that we can't have several display engine in
> > > parallel, if needs be. This has happened in the past already on
> > > Allwinner SoCs, so it's definitely something we should consider in
> > > the DT bindings, since we can't break them.
> >
> > IIRC, I proposed my driver before yours, and the DE2 is completely
> > different from the other display engines.
> > What you are telling is "add more code to already complex code and have
> > a big driver for all SoCs in each kernels".
> > I think it should be better to have small modules, each one treating
> > specific hardware, and to let only the needed code in the kernel memory
> > at startup time.
> >
> > > Until those are fixed, I cannot see how this driver can be merged,
> > > unfortunately.
> >
> > No problem. I just wanted to help people by giving the job I did on the
> > boards I have. My boards are working for almost one year, fine enough
> > for I use them as daily desktop computers. I don't want to spend one
> > more year for having my code in the Linux kernel: there are so much
> > other exciting things to do...
>
> And you're certainly welcome to contribute drivers to the kernel, that's
> always appreciated. Of course, to ensure a reasonable level of quality and
> consistency between drivers, the review process often requires changes to be
> made to the code being submitted. When it comes to drivers I mostly pay
> attention to DT bindings, userspace APIs and modification to common code.
> Driver code itself, as long as it's reasonably clean and doesn't impede
> development of other drivers or impact system security in an adverse way, is
> still important but maybe slightly less so. I'll defer to Maxime to come to an
> agreement on the multiple display engines in parallel problem as I'm not
> familiar with it for the Allwinner platforms.
The earlier Allwinner SoCs (with the old display engine), we had some
SoCs with multiple instances of the display engine and TCON (the
display engine roughly implementing the planes, the TCON the
CRTC. Roughly.). However, those were sharing some encoders (HDMI,
DSI) after that.
So we need to have a single DRM device taking care of the multiple
display engines, which essentialy means that we have to decouple the
DRM device from the display engine. This was done in the earlier
designs using an additional node with a list of phandles to the
display engines in the system, and obviously, I'd prefer to have some
consistency and reuse the same thing.
But the current approach doesn't work and will require some DT
modifications if that case happens again, which we can't do because of
the DT ABI.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 00/39] mtd: nand: denali: 2nd round of Denali NAND IP patch bomb
From: Masahiro Yamada @ 2016-12-01 9:15 UTC (permalink / raw)
To: Boris Brezillon
Cc: linux-mtd, devicetree, Linux Kernel Mailing List, Marek Vasut,
Brian Norris, Richard Weinberger, David Woodhouse,
Cyrille Pitchen, Rob Herring, Mark Rutland, Andy Shevchenko
In-Reply-To: <20161130091722.2e35f32a@bbrezillon>
Hi Boris,
2016-11-30 17:17 GMT+09:00 Boris Brezillon <boris.brezillon@free-electrons.com>:
>> [3]
>> Fix raw and oob callbacks.
>>
>> I asked in another thread,
>> the current driver just puts the physically accessed OOB data
>> into oob_poi, which is not a collection of ECC data.
>> Raw write/read() are wrong as well.
>
> That's all good things too.
>
>>
>> After fixing those, enable BBT scan by removing the following flag:
>> /* skip the scan for now until we have OOB read and write support */
>> chip->options |= NAND_SKIP_BBTSCAN;
>>
>
> Hm, here you have a problem. The layout you described replaces BBMs by
> payload data, thus preventing the BBM scan approach (or at least, it
> won't work with factory BBMs).
As I answered in another mail, the Denali IP expects BBMs
at the beginning of each OOB area (standard location).
They are protected from the ECC engine.
I just did not mention the BBM-reserved area
to make the story simpler.
So, after fixing oob read/write functions,
the driver will be able to enable BBT-scanning.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v3 3/4] [media] davinci: vpif_capture: get subdevs from DT
From: Laurent Pinchart @ 2016-12-01 9:16 UTC (permalink / raw)
To: Sakari Ailus
Cc: Kevin Hilman, linux-media-u79uwXL29TY76Z2rM5mHXA, Hans Verkuil,
devicetree-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Axel Haslam,
Bartosz Gołaszewski, Alexandre Bailon, David Lechner
In-Reply-To: <20161201075730.GP16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Hello,
On Thursday 01 Dec 2016 09:57:31 Sakari Ailus wrote:
> On Wed, Nov 30, 2016 at 04:14:11PM -0800, Kevin Hilman wrote:
> > Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
> >> On Wed, Nov 23, 2016 at 03:25:32PM -0800, Kevin Hilman wrote:
> >>> Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> writes:
> >>>> On Tue, Nov 22, 2016 at 07:52:43AM -0800, Kevin Hilman wrote:
> >>>>> Allow getting of subdevs from DT ports and endpoints.
> >>>>>
> >>>>> The _get_pdata() function was larely inspired by (i.e. stolen from)
> >>>>> am437x-vpfe.c
> >>>>>
> >>>>> Signed-off-by: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> >>>>> ---
> >>>>>
> >>>>> drivers/media/platform/davinci/vpif_capture.c | 130 +++++++++++++++-
> >>>>> include/media/davinci/vpif_types.h
> >>>>> | 9 +-
> >>>>> 2 files changed, 133 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/media/platform/davinci/vpif_capture.c
> >>>>> b/drivers/media/platform/davinci/vpif_capture.c index
> >>>>> 94ee6cf03f02..47a4699157e7 100644
> >>>>> --- a/drivers/media/platform/davinci/vpif_capture.c
> >>>>> +++ b/drivers/media/platform/davinci/vpif_capture.c
> >>>>> @@ -26,6 +26,8 @@
> >>>>> #include <linux/slab.h>
> >>>>>
> >>>>> #include <media/v4l2-ioctl.h>
> >>>>> +#include <media/v4l2-of.h>
> >>>>> +#include <media/i2c/tvp514x.h>
> >>>>
> >>>> Do you need this header?
> >>>
> >>> Yes, based on discussion with Hans, since there is no DT binding for
> >>> selecting the input pins of the TVP514x, I have to select it in the
> >>> driver, so I need the defines from this header. More on this below...
That's really ugly :-( The problem should be fixed properly instead of adding
one more offender.
> >>>>> #include "vpif.h"
> >>>>> #include "vpif_capture.h"
> >>>>> @@ -650,6 +652,10 @@ static int vpif_input_to_subdev(
> >>>>>
> >>>>> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
> >>>>>
> >>>>> + if (!chan_cfg)
> >>>>> + return -1;
> >>>>> + if (input_index >= chan_cfg->input_count)
> >>>>> + return -1;
> >>>>> subdev_name = chan_cfg->inputs[input_index].subdev_name;
> >>>>> if (subdev_name == NULL)
> >>>>> return -1;
> >>>>> @@ -657,7 +663,7 @@ static int vpif_input_to_subdev(
> >>>>> /* loop through the sub device list to get the sub device info
> >>>>> */
> >>>>> for (i = 0; i < vpif_cfg->subdev_count; i++) {
> >>>>> subdev_info = &vpif_cfg->subdev_info[i];
> >>>>> - if (!strcmp(subdev_info->name, subdev_name))
> >>>>> + if (subdev_info && !strcmp(subdev_info->name,
> >>>>> subdev_name))
> >>>>> return i;
> >>>>> }
> >>>>> return -1;
> >>>>> @@ -1327,6 +1333,21 @@ static int vpif_async_bound(struct
> >>>>> v4l2_async_notifier *notifier,> >> >>
> >>>>> {
> >>>>> int i;
> >>>>>
> >>>>> + for (i = 0; i < vpif_obj.config->asd_sizes[0]; i++) {
> >>>>> + struct v4l2_async_subdev *_asd = vpif_obj.config
> >>>>> ->asd[i];
> >>>>> + const struct device_node *node = _asd->match.of.node;
> >>>>> +
> >>>>> + if (node == subdev->of_node) {
> >>>>> + vpif_obj.sd[i] = subdev;
> >>>>> + vpif_obj.config->chan_config
> >>>>> ->inputs[i].subdev_name =
> >>>>> + (char *)subdev->of_node->full_name;
Can subdev_name be made const instead of blindly casting the full_name pointer
? If not this is probably unsafe, and if yes it should be done :-)
> >>>>> + vpif_dbg(2, debug,
> >>>>> + "%s: setting input %d subdev_name =
> >>>>> %s\n",
> >>>>> + __func__, i, subdev->of_node
> >>>>> ->full_name);
> >>>>> + return 0;
> >>>>> + }
> >>>>> + }
> >>>>> +
> >>>>> for (i = 0; i < vpif_obj.config->subdev_count; i++)
> >>>>> if (!strcmp(vpif_obj.config->subdev_info[i].name,
> >>>>> subdev->name)) {
> >>>>> @@ -1422,6 +1443,110 @@ static int vpif_async_complete(struct
> >>>>> v4l2_async_notifier *notifier)
> >>>>> return vpif_probe_complete();
> >>>>> }
> >>>>>
> >>>>> +static struct vpif_capture_config *
> >>>>> +vpif_capture_get_pdata(struct platform_device *pdev)
> >>>>> +{
> >>>>> + struct device_node *endpoint = NULL;
> >>>>> + struct v4l2_of_endpoint bus_cfg;
> >>>>> + struct vpif_capture_config *pdata;
> >>>>> + struct vpif_subdev_info *sdinfo;
> >>>>> + struct vpif_capture_chan_config *chan;
> >>>>> + unsigned int i;
> >>>>> +
> >>>>> + dev_dbg(&pdev->dev, "vpif_get_pdata\n");
> >>>>> +
> >>>>> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> >>>>> + return pdev->dev.platform_data;
> >>>>> +
> >>>>> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> >>>>> + if (!pdata)
> >>>>> + return NULL;
> >>>>> + pdata->subdev_info =
> >>>>> + devm_kzalloc(&pdev->dev, sizeof(*pdata->subdev_info) *
> >>>>> + VPIF_CAPTURE_MAX_CHANNELS, GFP_KERNEL);
> >>>>> +
> >>>>> + if (!pdata->subdev_info)
> >>>>> + return NULL;
> >>>>> + dev_dbg(&pdev->dev, "%s\n", __func__);
> >>>>> +
> >>>>> + for (i = 0; ; i++) {
> >>>>> + struct device_node *rem;
> >>>>> + unsigned int flags;
> >>>>> + int err;
> >>>>> +
> >>>>> + endpoint = of_graph_get_next_endpoint(pdev
> >>>>> ->dev.of_node,
> >>>>> + endpoint);
> >>>>> + if (!endpoint)
> >>>>> + break;
> >>>>> +
> >>>>> + sdinfo = &pdata->subdev_info[i];
> >>>>
> >>>> subdev_info[] has got VPIF_CAPTURE_MAX_CHANNELS entries only.
> >>>
> >>> Right, I need to make the loop only go for a max of
> >>> VPIF_CAPTURE_MAX_CHANNELS iterations.
> >>>
> >>>>> + chan = &pdata->chan_config[i];
> >>>>> + chan->inputs = devm_kzalloc(&pdev->dev,
> >>>>> + sizeof(*chan->inputs) *
> >>>>> + VPIF_DISPLAY_MAX_CHANNELS,
> >>>>> + GFP_KERNEL);
> >>>>> +
> >>>>> + chan->input_count++;
> >>>>> + chan->inputs[i].input.type = V4L2_INPUT_TYPE_CAMERA;
> >>>>
> >>>> I wonder what's the purpose of using index i on this array as well.
> >>>
> >>> The number of endpoints in DT is the number of input channels
> >>> configured (up to a max of VPIF_CAPTURE_MAX_CHANNELS.)
> >>>
> >>>> If you use that to access a corresponding entry in a different array,
> >>>> I'd just create a struct that contains the port configuration and the
> >>>> async sub-device. The omap3isp driver does that, for instance; see
> >>>> isp_of_parse_nodes() in drivers/media/platform/omap3isp/isp.c if
> >>>> you're interested. Up to you.
> >>>
> >>> OK, I'll have a look at that driver. The goal here with this series is
> >>> just to get this working with DT, but also not break the existing
> >>> legacy platform_device support, so I'm trying not to mess with the
> >>> driver-interal data structures too much.
> >>
> >> Ack.
> >>
> >>>>> + chan->inputs[i].input.std = V4L2_STD_ALL;
> >>>>> + chan->inputs[i].input.capabilities = V4L2_IN_CAP_STD;
> >>>>> +
> >>>>> + /* FIXME: need a new property? ch0:composite ch1:
> >>>>> s-video */
> >>>>> + if (i == 0)
> >>>>
> >>>> Can you assume that the first endopoint has got a particular kind of
> >>>> input? What if it's not connected?
> >>>
> >>> On all the boards I know of (there aren't many using this SoC), it's a
> >>> safe assumption.
> >>>
> >>>> If this is a different physical port (not in the meaning another) in
> >>>> the device, I'd use the reg property for this. Please see
> >>>> Documentation/devicetree/bindings/media/video-interfaces.txt .
> >>>
> >>> My understanding (which is admittedly somewhat fuzzy) of the TVP514x is
> >>> that it's not physically a different port. Instead, it's just telling
> >>> the TVP514x which pin(s) will be active inputs (and what kind of signal
> >>> will be present.)
> >>>
> >>> I'm open to a better way to describe this input select from DT, but
> >>> based on what I heard from Hans, there isn't currently a good way to do
> >>> that except for in the driver:
> >>> (c.f. https://marc.info/?l=linux-arm-kernel&m=147887871615788)
> >>>
> >>> Based on further discussion in that thread, it sounds like there may be
> >>> a way forward coming soon, and I'll be glad to switch to that when it
> >>> arrives.
I'm afraid I have to disappoint Hans here, I don't have code for that yet.
> >> I'm not sure that properly supporting connectors will provide any help
> >> here.
> >>
> >> Looking at the s_routing() API, it's the calling driver that has to be
> >> aware of sub-device specific function parameters. As such it's not a
> >> very good idea to require that a driver is aware of the value range of
> >> another driver's parameter. I wonder if a simple enumeration interface
> >> would help here --- if I understand correctly, the purpose is just to
> >> provide a way to choose the input using VIDIOC_S_INPUT.
> >>
> >> I guess that's somehow ok as long as you have no other combinations of
> >> these devices but this is hardly future-proof. (And certainly not a
> >> problem created by this patch.)
> >
> > Yeah, this is far from future proof.
> >
> >> It'd be still nice to fix that as presumably we don't have the option of
> >> reworking how we expect the device tree to look like.
> >
> > Agreed.
> >
> > I'm just hoping someone can shed som light on "how we expect the device
> > tree to look". ;)
>
> :-)
>
> For the tvp514x, do you need more than a single endpoint on the receiver
> side? Does the input that's selected affect the bus parameters?
>
> If it doesn't, you could create a custom endpoint property for the possible
> input values. The s_routing() really should be fixed though, but that could
> be postponed I guess. There are quite a few drivers using it.
There's two ways to look at s_routing() in my opinion, as the calling driver
should really not hardcode any knowledge specific to a particular subdev. We
can either have the calling driver discover the possible routing options at
runtime through the subdev API, or modify the s_routing() API.
--
Regards,
Laurent Pinchart
--
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 v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Laurent Pinchart @ 2016-12-01 9:19 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161201091313.th7nucjmvtuolqza@lukather>
Hi Maxime,
On Thursday 01 Dec 2016 10:13:13 Maxime Ripard wrote:
> On Wed, Nov 30, 2016 at 12:12:55PM +0200, Laurent Pinchart wrote:
> >> More, it is not sure that the bridge/DW code would work with Allwinner's
> >> SoCs.
> >
> > If it doesn't work and can't be made to work in a non-invasive way they it
> > should certainly not be used :-)
>
> Even if the register layout is completely scrambled, as long as the
> bits themselves aren't (and by comparing the two drivers it looks like
> they haven't changed), you can easily deal with that using the
> regmap_fields, with the two implementations (the original one and the
> scrambled one) providing their register map that way, and the driver
> code using whatever has been provided.
Looking at https://linux-sunxi.org/DWC_HDMI_Controller#DWC_HDMI_Controller it
seems that an address remapping function could be used.
> >> Eventually, I went the same way as omap/hdmi5: different driver.
> >
> > I might try to fix that for OMAP5 at some point, we'll see.
>
> For complex drivers that have already a driver written and a lot of
> testing that already happened, I don't think duplication is a smart
> move.
>
> >>> - And finally the fact that we can't have several display engine in
> >>> parallel, if needs be. This has happened in the past already on
> >>> Allwinner SoCs, so it's definitely something we should consider in
> >>> the DT bindings, since we can't break them.
> >>
> >> IIRC, I proposed my driver before yours, and the DE2 is completely
> >> different from the other display engines.
> >> What you are telling is "add more code to already complex code and have
> >> a big driver for all SoCs in each kernels".
> >> I think it should be better to have small modules, each one treating
> >> specific hardware, and to let only the needed code in the kernel memory
> >> at startup time.
> >>
> >>> Until those are fixed, I cannot see how this driver can be merged,
> >>> unfortunately.
> >>
> >> No problem. I just wanted to help people by giving the job I did on the
> >> boards I have. My boards are working for almost one year, fine enough
> >> for I use them as daily desktop computers. I don't want to spend one
> >> more year for having my code in the Linux kernel: there are so much
> >> other exciting things to do...
> >
> > And you're certainly welcome to contribute drivers to the kernel, that's
> > always appreciated. Of course, to ensure a reasonable level of quality and
> > consistency between drivers, the review process often requires changes to
> > be made to the code being submitted. When it comes to drivers I mostly
> > pay attention to DT bindings, userspace APIs and modification to common
> > code. Driver code itself, as long as it's reasonably clean and doesn't
> > impede development of other drivers or impact system security in an
> > adverse way, is still important but maybe slightly less so. I'll defer to
> > Maxime to come to an agreement on the multiple display engines in
> > parallel problem as I'm not familiar with it for the Allwinner platforms.
>
> The earlier Allwinner SoCs (with the old display engine), we had some
> SoCs with multiple instances of the display engine and TCON (the
> display engine roughly implementing the planes, the TCON the
> CRTC. Roughly.). However, those were sharing some encoders (HDMI,
> DSI) after that.
>
> So we need to have a single DRM device taking care of the multiple
> display engines, which essentialy means that we have to decouple the
> DRM device from the display engine. This was done in the earlier
> designs using an additional node with a list of phandles to the
> display engines in the system, and obviously, I'd prefer to have some
> consistency and reuse the same thing.
>
> But the current approach doesn't work and will require some DT
> modifications if that case happens again, which we can't do because of
> the DT ABI.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] ARM: dts: sunxi: Add num-cs for A20 spi nodes
From: Maxime Ripard @ 2016-12-01 9:21 UTC (permalink / raw)
To: Emmanuel Vadot
Cc: mark.rutland, devicetree, linux, linux-kernel, wens, robh+dt,
linux-arm-kernel
In-Reply-To: <20161125220752.c989c85e01ed202be0485c78@bidouilliste.com>
[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]
Hi Emmanuel,
On Fri, Nov 25, 2016 at 10:07:52PM +0100, Emmanuel Vadot wrote:
> On Fri, 25 Nov 2016 16:20:47 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>
> > On Thu, Nov 24, 2016 at 09:05:09PM +0100, Emmanuel Vadot wrote:
> > > On Thu, 24 Nov 2016 20:55:17 +0100
> > > Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > >
> > > > On Tue, Nov 22, 2016 at 06:06:16PM +0100, Emmanuel Vadot wrote:
> > > > > The spi0 controller on the A20 have up to 4 CS (Chip Select) while the
> > > > > others three only have 1.
> > > > > Add the num-cs property to each node.
> > > > >
> > > > > Signed-off-by: Emmanuel Vadot <manu@bidouilliste.com>
> > > >
> > > > I don't think we have any code that uses it at the moment. What is the
> > > > rationale behind this patch?
> > > >
> > > > Thanks!
> > > > Maxime
> > > >
> > > > --
> > > > Maxime Ripard, Free Electrons
> > > > Embedded Linux and Kernel engineering
> > > > http://free-electrons.com
> > >
> > > Hi Maxime,
> > >
> > > If num-cs isn't present nothing prevent to start a transfer with a
> > > non-valid CS pin, resulting in an error.
> > > num-cs are default property especially made for this and a SPI driver
> > > should try to get the property at probe/attach time.
> >
> > Yes, but as far as I know, our driver doesn't. I'm all in for having
> > support for that in our driver, but without it, that patch is kind of
> > useless.
>
> Yes the Linux driver doesn't use it but my upcoming one for FreeBSD
> uses it. So it is not useless for downstream user of DTS.
Ah, I didn't know this was for FreeBSD. So you started to use our DTs,
or do you have some modifications to it? How does that work?
Anyway, the fact that it isn't used by our driver at the moment and
that it's meant for other OSes should be mentionned in the commit log.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* How should we group related devices in DT ? (was Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support)
From: Laurent Pinchart @ 2016-12-01 9:28 UTC (permalink / raw)
To: Maxime Ripard
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161201091313.th7nucjmvtuolqza@lukather>
Hello,
On Thursday 01 Dec 2016 10:13:13 Maxime Ripard wrote:
[snip]
> The earlier Allwinner SoCs (with the old display engine), we had some
> SoCs with multiple instances of the display engine and TCON (the
> display engine roughly implementing the planes, the TCON the
> CRTC. Roughly.). However, those were sharing some encoders (HDMI,
> DSI) after that.
>
> So we need to have a single DRM device taking care of the multiple
> display engines, which essentialy means that we have to decouple the
> DRM device from the display engine. This was done in the earlier
> designs using an additional node with a list of phandles to the
> display engines in the system, and obviously, I'd prefer to have some
> consistency and reuse the same thing.
I believe this problem isn't limited to sunxi and should be addressed in a
more generic way. How should we describe in the device tree that multiple
instances of a device unrelated from a control point of view are related at
the hardware level ? There are multiple reasons why we need this, and here are
a few.
- As described above, unrelated display controller instances that share
encoders at their output need to be exposed to userspace as a single DRM
device. This is also the case on Renesas platforms (where the display engines
are independent except for the "small" detail that output routing is
controlled through the first display engine).
- On Renesas platforms again a radio-related SPI receiver has multiple
independent channels that each have their own registers, clocks and
interrupts, but share the same physical clock and sync pins. They are used to
receive multiple channels of the same data stream and must be exposed as a
single V4L2 device to userspace. A generic DT binding RFC is available at
http://www.spinics.net/lists/devicetree/msg152414.html.
> But the current approach doesn't work and will require some DT
> modifications if that case happens again, which we can't do because of
> the DT ABI.
--
Regards,
Laurent Pinchart
--
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/3] ARM: dts: sunxi: add support for Orange Pi Zero board
From: Maxime Ripard @ 2016-12-01 9:36 UTC (permalink / raw)
To: André Przywara
Cc: Mark Rutland, devicetree@vger.kernel.org, Vishnu Patekar,
Arnd Bergmann, Jonathan Corbet, linux-doc@vger.kernel.org,
Russell King, linux-kernel@vger.kernel.org, Hans de Goede,
Chen-Yu Tsai, Icenowy Zheng, linux-arm-kernel@lists.infradead.org
In-Reply-To: <cdfd3c65-d473-badb-ea6a-035f7ab79217@arm.com>
[-- Attachment #1.1: Type: text/plain, Size: 1083 bytes --]
On Mon, Nov 28, 2016 at 12:29:07AM +0000, André Przywara wrote:
> > Something more interesting happened.
> >
> > Xunlong made a add-on board for Orange Pi Zero, which exposes the
> > two USB Controllers exported at expansion bus as USB Type-A
> > connectors.
> >
> > Also it exposes a analog A/V jack and a microphone.
> >
> > Should I enable {e,o}hci{2.3} in the device tree?
>
> Actually we should do this regardless of this extension board. The USB
> pins are not multiplexed and are exposed on user accessible pins (just
> not soldered, but that's a detail), so I think they qualify for DT
> enablement. And even if a user can't use them, it doesn't hurt to have
> them (since they are not multiplexed).
My main concern about this is that we'll leave regulators enabled by
default, for a minority of users. And that minority will prevent to do
a proper power management when the times come since we'll have to keep
that behaviour forever.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v7 0/8] drm: sun8i: Add DE2 HDMI video support
From: Maxime Ripard @ 2016-12-01 9:42 UTC (permalink / raw)
To: Laurent Pinchart
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Jean-Francois Moine,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <2084988.kISO4Quil7@avalon>
[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]
On Thu, Dec 01, 2016 at 11:19:56AM +0200, Laurent Pinchart wrote:
> Hi Maxime,
>
> On Thursday 01 Dec 2016 10:13:13 Maxime Ripard wrote:
> > On Wed, Nov 30, 2016 at 12:12:55PM +0200, Laurent Pinchart wrote:
> > >> More, it is not sure that the bridge/DW code would work with Allwinner's
> > >> SoCs.
> > >
> > > If it doesn't work and can't be made to work in a non-invasive way they it
> > > should certainly not be used :-)
> >
> > Even if the register layout is completely scrambled, as long as the
> > bits themselves aren't (and by comparing the two drivers it looks like
> > they haven't changed), you can easily deal with that using the
> > regmap_fields, with the two implementations (the original one and the
> > scrambled one) providing their register map that way, and the driver
> > code using whatever has been provided.
>
> Looking at https://linux-sunxi.org/DWC_HDMI_Controller#DWC_HDMI_Controller it
> seems that an address remapping function could be used.
Even better.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH 1/4] mmc: mediatek: Fix CMD6 timeout issue
From: Ulf Hansson @ 2016-12-01 9:51 UTC (permalink / raw)
To: Yong Mao
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
YH Huang, Nicolas Boichat, Mathias Nyman, srv_heupstream,
Catalin Marinas, Will Deacon, Douglas Anderson,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Chunfeng Yun, Rob Herring, Geert Uytterhoeven,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Philipp Zabel, Matthias Brugger,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Eddie Huang,
Chaotian Jing <chaot>
In-Reply-To: <1478585341-6749-2-git-send-email-yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
On 8 November 2016 at 07:08, Yong Mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> From: yong mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> When initializing EMMC, after switch to HS400,
> it will issue CMD6 to change ext_csd, if first CMD6 got CRC
> error, the repeat CMD6 may get timeout, that's
> because SDCBSY was cleared by msdc_reset_hw()
Sorry for the delay!
We have recently been re-working the sequence for how to deal more
properly with CMD6 in the mmc core.
The changes done so far should mostly concern switches to HS and HS
DDR, but I think you should run a re-test to make sure you still hit
the same problems.
>
> Signed-off-by: Yong Mao <yong.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Chaotian Jing <chaotian.jing-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
> drivers/mmc/host/mtk-sd.c | 77 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 51 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 84e9afc..b29683b 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -826,6 +826,15 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
> return true;
> }
>
> +static int msdc_card_busy(struct mmc_host *mmc)
> +{
> + struct msdc_host *host = mmc_priv(mmc);
> + u32 status = readl(host->base + MSDC_PS);
> +
> + /* check if data0 is low */
> + return !(status & BIT(16));
> +}
> +
> /* It is the core layer's responsibility to ensure card status
> * is correct before issue a request. but host design do below
> * checks recommended.
Hmm. Why?
I think you should rely on the mmc core to invoke the ->card_busy()
ops instead. The core knows better when it's needed.
Perhaps that may also resolve some of these issues for you!?
> @@ -835,10 +844,20 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
> {
> /* The max busy time we can endure is 20ms */
> unsigned long tmo = jiffies + msecs_to_jiffies(20);
> + u32 count = 0;
> +
> + if (in_interrupt()) {
> + while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> + (count < 1000)) {
> + udelay(1);
> + count++;
This seems like a bad idea, "busy-wait" in irq context is never a good idea.
> + }
> + } else {
> + while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> + time_before(jiffies, tmo))
> + cpu_relax();
> + }
>
> - while ((readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) &&
> - time_before(jiffies, tmo))
> - cpu_relax();
> if (readl(host->base + SDC_STS) & SDC_STS_CMDBUSY) {
> dev_err(host->dev, "CMD bus busy detected\n");
> host->error |= REQ_CMD_BUSY;
> @@ -846,17 +865,35 @@ static inline bool msdc_cmd_is_ready(struct msdc_host *host,
> return false;
> }
>
> - if (mmc_resp_type(cmd) == MMC_RSP_R1B || cmd->data) {
> - tmo = jiffies + msecs_to_jiffies(20);
> - /* R1B or with data, should check SDCBUSY */
> - while ((readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) &&
> - time_before(jiffies, tmo))
> - cpu_relax();
> - if (readl(host->base + SDC_STS) & SDC_STS_SDCBUSY) {
> - dev_err(host->dev, "Controller busy detected\n");
> - host->error |= REQ_CMD_BUSY;
> - msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> - return false;
> + if (cmd->opcode != MMC_SEND_STATUS) {
> + count = 0;
> + /* Consider that CMD6 crc error before card was init done,
> + * mmc_retune() will return directly as host->card is null.
> + * and CMD6 will retry 3 times, must ensure card is in transfer
> + * state when retry.
> + */
> + tmo = jiffies + msecs_to_jiffies(60 * 1000);
> + while (1) {
> + if (msdc_card_busy(host->mmc)) {
> + if (in_interrupt()) {
> + udelay(1);
> + count++;
> + } else {
> + msleep_interruptible(10);
> + }
> + } else {
> + break;
> + }
> + /* Timeout if the device never
> + * leaves the program state.
> + */
> + if (count > 1000 || time_after(jiffies, tmo)) {
> + pr_err("%s: Card stuck in programming state! %s\n",
> + mmc_hostname(host->mmc), __func__);
> + host->error |= REQ_CMD_BUSY;
> + msdc_cmd_done(host, MSDC_INT_CMDTMO, mrq, cmd);
> + return false;
> + }
This hole new code is a hack, that shouldn't be needed in the host driver.
I think we need to investigate and fix the issue in the mmc core
layer, to make this work for your host driver. That instead of doing a
work around in the host.
> }
> }
> return true;
> @@ -1070,18 +1107,6 @@ static int msdc_ops_switch_volt(struct mmc_host *mmc, struct mmc_ios *ios)
> return ret;
> }
>
> -static int msdc_card_busy(struct mmc_host *mmc)
> -{
> - struct msdc_host *host = mmc_priv(mmc);
> - u32 status = readl(host->base + MSDC_PS);
> -
> - /* check if any pin between dat[0:3] is low */
> - if (((status >> 16) & 0xf) != 0xf)
> - return 1;
> -
> - return 0;
> -}
> -
> static void msdc_request_timeout(struct work_struct *work)
> {
> struct msdc_host *host = container_of(work, struct msdc_host,
> --
> 1.7.9.5
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] arm64: dts: juno: Correct PCI IO window
From: Sudeep Holla @ 2016-12-01 9:58 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Sudeep Holla, Jeremy Linton, devicetree-u79uwXL29TY76Z2rM5mHXA,
mark.rutland-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
catalin.marinas-5wv7dgnIgG8, liviu.dudau-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <7823573.FNB8ayVOnQ@wuerfel>
On 30/11/16 22:51, Arnd Bergmann wrote:
> On Wednesday, November 30, 2016 4:29:35 PM CET Sudeep Holla wrote:
>> Hi Jeremy,
>>
>> On 29/11/16 20:45, Jeremy Linton wrote:
>>> The PCIe root complex on Juno translates the MMIO mapped
>>> at 0x5f800000 to the PIO address range starting at 0
>>> (which is common because PIO addresses are generally < 64k).
>>> Correct the DT to reflect this.
>>>
>>
>> I have another DT fix that I have asked ARM-SoC guys to pick up directly
>> from the list. If that doesn't happen, I will send PR including both.
>>
>> If that happens then we need to send this to them to be picked directly.
>> At this point I want to wait for couple of days to avoid confusion.
>
> I ended up taking the other one for v4.10, but this one seems more
> important so I applied it for v4.9.
>
> Let me know if you disagree with the priorities, as I plan to send out
> the last 4.9 fixes pull request to Linus tomorrow.
>
No that's fine.
--
Regards,
Sudeep
--
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] ARM: dts: sunxi: Add num-cs for A20 spi nodes
From: Emmanuel Vadot @ 2016-12-01 10:24 UTC (permalink / raw)
To: Maxime Ripard
Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
wens-jdAy2FN1RRM, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161201092150.rlo5skxd6elovlgq@lukather>
Hi Maxime,
On Thu, 1 Dec 2016 10:21:50 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi Emmanuel,
>
> On Fri, Nov 25, 2016 at 10:07:52PM +0100, Emmanuel Vadot wrote:
> > On Fri, 25 Nov 2016 16:20:47 +0100
> > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> >
> > > On Thu, Nov 24, 2016 at 09:05:09PM +0100, Emmanuel Vadot wrote:
> > > > On Thu, 24 Nov 2016 20:55:17 +0100
> > > > Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > > >
> > > > > On Tue, Nov 22, 2016 at 06:06:16PM +0100, Emmanuel Vadot wrote:
> > > > > > The spi0 controller on the A20 have up to 4 CS (Chip Select) while the
> > > > > > others three only have 1.
> > > > > > Add the num-cs property to each node.
> > > > > >
> > > > > > Signed-off-by: Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org>
> > > > >
> > > > > I don't think we have any code that uses it at the moment. What is the
> > > > > rationale behind this patch?
> > > > >
> > > > > Thanks!
> > > > > Maxime
> > > > >
> > > > > --
> > > > > Maxime Ripard, Free Electrons
> > > > > Embedded Linux and Kernel engineering
> > > > > http://free-electrons.com
> > > >
> > > > Hi Maxime,
> > > >
> > > > If num-cs isn't present nothing prevent to start a transfer with a
> > > > non-valid CS pin, resulting in an error.
> > > > num-cs are default property especially made for this and a SPI driver
> > > > should try to get the property at probe/attach time.
> > >
> > > Yes, but as far as I know, our driver doesn't. I'm all in for having
> > > support for that in our driver, but without it, that patch is kind of
> > > useless.
> >
> > Yes the Linux driver doesn't use it but my upcoming one for FreeBSD
> > uses it. So it is not useless for downstream user of DTS.
>
> Ah, I didn't know this was for FreeBSD. So you started to use our DTs,
> or do you have some modifications to it? How does that work?
Yes we use the DTS from linux from quite some times now. We're
currently synced with 4.7-ish.
We either use them directly or modify them according to our needs and
driver support.
> Anyway, the fact that it isn't used by our driver at the moment and
> that it's meant for other OSes should be mentionned in the commit log.
Yeah I understand, I'll send a v2 with this in the commit log.
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Emmanuel Vadot <manu-xXdDKFdH5B3kFDPD4ZthVA@public.gmane.org> <manu-h+KGxgPPiopAfugRpC6u6w@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 0/5] Add support for the Armada 3700 SPI controller
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
The Marvell Armada 3700 SoC includes an SPI controller. This controller
supports up to 4 SPI slave devices, with dedicated chip selects, CPIO or
FIFO mode with DMA or CPU transfers and different SPI transfer modes
(Standard single, Dual or Quad).
This set of patches adds a basic support for the CPIO mode, then it
enables the FIFO mode (CPU-side only, DMA not supported yet). It also
adds the required definitions of the spi nodes to the devicetree.
Romain Perier (5):
spi: Add basic support for Armada 3700 SPI Controller
spi: armada-3700: Add support for the FIFO mode
spi: armada-3700: Add documentation for the Armada 3700 SPI Controller
arm64: dts: marvell: Add definition of SPI controller for Armada 3700
arm64: dts: marvell: Enable spi0 on the board Armada-3720-db
.../devicetree/bindings/spi/spi-armada-3700.txt | 25 +
arch/arm64/boot/dts/marvell/armada-3720-db.dts | 30 +
arch/arm64/boot/dts/marvell/armada-37xx.dtsi | 11 +
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-armada-3700.c | 1046 ++++++++++++++++++++
6 files changed, 1120 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-armada-3700.txt
create mode 100644 drivers/spi/spi-armada-3700.c
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 1/5] spi: Add basic support for Armada 3700 SPI Controller
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Marvell Armada 3700 SoC comprises an SPI Controller. This Controller
supports up to 4 SPI slave devices, with dedicated chip selects, supports
SPI mode 0/1/2 and 3, CPIO or Fifo mode with DMA transfers and different
SPI transfer mode (Single, Dual or Quad).
This commit adds basic driver support for CPIO mode and single SPI
transfer. In this mode, the CPU asserts cs, outputs or inputs data from
the current SPI device. Data transfers are copied by 1 or 4 bytes using
the SPI registers.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v3:
- Fixed wrong variable passed as MODULE_DEVICE_TABLE
- Added missing null terminated entry in a3700_spi_dt_ids
- Added the tag "Tested-by" by Gregory
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-armada-3700.c | 652 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 660 insertions(+)
create mode 100644 drivers/spi/spi-armada-3700.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b799547..6ade1ca 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -67,6 +67,13 @@ config SPI_ATH79
This enables support for the SPI controller present on the
Atheros AR71XX/AR724X/AR913X SoCs.
+config SPI_ARMADA_3700
+ tristate "Marvell Armada 3700 SPI Controller"
+ depends on ARCH_MVEBU && OF
+ help
+ This enables support for the SPI controller present on the
+ Marvell Armada 3700 SoCs.
+
config SPI_ATMEL
tristate "Atmel SPI Controller"
depends on HAS_DMA
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index aa939d9..140ca45 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST) += spi-loopback-test.o
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_ALTERA) += spi-altera.o
+obj-$(CONFIG_SPI_ARMADA_3700) += spi-armada-3700.o
obj-$(CONFIG_SPI_ATMEL) += spi-atmel.o
obj-$(CONFIG_SPI_ATH79) += spi-ath79.o
obj-$(CONFIG_SPI_AU1550) += spi-au1550.o
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
new file mode 100644
index 0000000..4115685
--- /dev/null
+++ b/drivers/spi/spi-armada-3700.c
@@ -0,0 +1,652 @@
+/*
+ * Marvell Armada-3700 SPI controller driver
+ *
+ * Copyright (C) 2016 Marvell Ltd.
+ *
+ * Author: Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
+ * Author: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@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 version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME "armada_3700_spi"
+
+#define A3700_SPI_TIMEOUT 10
+
+/* SPI Register Offest */
+#define A3700_SPI_IF_CTRL_REG 0x00
+#define A3700_SPI_IF_CFG_REG 0x04
+#define A3700_SPI_DATA_OUT_REG 0x08
+#define A3700_SPI_DATA_IN_REG 0x0C
+#define A3700_SPI_IF_INST_REG 0x10
+#define A3700_SPI_IF_ADDR_REG 0x14
+#define A3700_SPI_IF_RMODE_REG 0x18
+#define A3700_SPI_IF_HDR_CNT_REG 0x1C
+#define A3700_SPI_IF_DIN_CNT_REG 0x20
+#define A3700_SPI_IF_TIME_REG 0x24
+#define A3700_SPI_INT_STAT_REG 0x28
+#define A3700_SPI_INT_MASK_REG 0x2C
+
+/* A3700_SPI_IF_CTRL_REG */
+#define A3700_SPI_EN BIT(16)
+#define A3700_SPI_ADDR_NOT_CONFIG BIT(12)
+#define A3700_SPI_WFIFO_OVERFLOW BIT(11)
+#define A3700_SPI_WFIFO_UNDERFLOW BIT(10)
+#define A3700_SPI_RFIFO_OVERFLOW BIT(9)
+#define A3700_SPI_RFIFO_UNDERFLOW BIT(8)
+#define A3700_SPI_WFIFO_FULL BIT(7)
+#define A3700_SPI_WFIFO_EMPTY BIT(6)
+#define A3700_SPI_RFIFO_FULL BIT(5)
+#define A3700_SPI_RFIFO_EMPTY BIT(4)
+#define A3700_SPI_WFIFO_RDY BIT(3)
+#define A3700_SPI_RFIFO_RDY BIT(2)
+#define A3700_SPI_XFER_RDY BIT(1)
+#define A3700_SPI_XFER_DONE BIT(0)
+
+/* A3700_SPI_IF_CFG_REG */
+#define A3700_SPI_WFIFO_THRS BIT(28)
+#define A3700_SPI_RFIFO_THRS BIT(24)
+#define A3700_SPI_AUTO_CS BIT(20)
+#define A3700_SPI_DMA_RD_EN BIT(18)
+#define A3700_SPI_FIFO_MODE BIT(17)
+#define A3700_SPI_SRST BIT(16)
+#define A3700_SPI_XFER_START BIT(15)
+#define A3700_SPI_XFER_STOP BIT(14)
+#define A3700_SPI_INST_PIN BIT(13)
+#define A3700_SPI_ADDR_PIN BIT(12)
+#define A3700_SPI_DATA_PIN1 BIT(11)
+#define A3700_SPI_DATA_PIN0 BIT(10)
+#define A3700_SPI_FIFO_FLUSH BIT(9)
+#define A3700_SPI_RW_EN BIT(8)
+#define A3700_SPI_CLK_POL BIT(7)
+#define A3700_SPI_CLK_PHA BIT(6)
+#define A3700_SPI_BYTE_LEN BIT(5)
+#define A3700_SPI_CLK_PRESCALE BIT(0)
+#define A3700_SPI_CLK_PRESCALE_MASK (0x1f)
+
+#define A3700_SPI_WFIFO_THRS_BIT 28
+#define A3700_SPI_RFIFO_THRS_BIT 24
+#define A3700_SPI_FIFO_THRS_MASK 0x7
+
+#define A3700_SPI_DATA_PIN_MASK 0x3
+
+/* A3700_SPI_IF_HDR_CNT_REG */
+#define A3700_SPI_DUMMY_CNT_BIT 12
+#define A3700_SPI_DUMMY_CNT_MASK 0x7
+#define A3700_SPI_RMODE_CNT_BIT 8
+#define A3700_SPI_RMODE_CNT_MASK 0x3
+#define A3700_SPI_ADDR_CNT_BIT 4
+#define A3700_SPI_ADDR_CNT_MASK 0x7
+#define A3700_SPI_INSTR_CNT_BIT 0
+#define A3700_SPI_INSTR_CNT_MASK 0x3
+
+/* A3700_SPI_IF_TIME_REG */
+#define A3700_SPI_CLK_CAPT_EDGE BIT(7)
+
+struct a3700_spi {
+ struct spi_master *master;
+ void __iomem *base;
+ struct clk *clk;
+ unsigned int irq;
+ unsigned int flags;
+ bool last_xfer;
+ const u8 *tx_buf;
+ u8 *rx_buf;
+ size_t buf_len;
+ u8 byte_len;
+ u32 wait_mask;
+ struct completion done;
+};
+
+static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
+{
+ return readl(a3700_spi->base + offset);
+}
+
+static void spireg_write(struct a3700_spi *a3700_spi, u32 offset, u32 data)
+{
+ writel(data, a3700_spi->base + offset);
+}
+
+static void a3700_spi_auto_cs_unset(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_AUTO_CS;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_activate_cs(struct a3700_spi *a3700_spi, unsigned int cs)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ val |= (A3700_SPI_EN << cs);
+ spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static void a3700_spi_deactivate_cs(struct a3700_spi *a3700_spi,
+ unsigned int cs)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ val &= ~(A3700_SPI_EN << cs);
+ spireg_write(a3700_spi, A3700_SPI_IF_CTRL_REG, val);
+}
+
+static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
+ unsigned int pin_mode)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_INST_PIN | A3700_SPI_ADDR_PIN);
+ val &= ~(A3700_SPI_DATA_PIN0 | A3700_SPI_DATA_PIN1);
+
+ switch (pin_mode) {
+ case 1:
+ break;
+ case 2:
+ val |= A3700_SPI_DATA_PIN0;
+ break;
+ case 4:
+ val |= A3700_SPI_DATA_PIN1;
+ break;
+ default:
+ dev_err(&a3700_spi->master->dev, "wrong pin mode %u", pin_mode);
+ return -EINVAL;
+ }
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ return 0;
+}
+
+static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_FIFO_MODE;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_mode_set(struct a3700_spi *a3700_spi,
+ unsigned int mode_bits)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+
+ if (mode_bits & SPI_CPOL)
+ val |= A3700_SPI_CLK_POL;
+ else
+ val &= ~A3700_SPI_CLK_POL;
+
+ if (mode_bits & SPI_CPHA)
+ val |= A3700_SPI_CLK_PHA;
+ else
+ val &= ~A3700_SPI_CLK_PHA;
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_clock_set(struct a3700_spi *a3700_spi,
+ unsigned int speed_hz, u16 mode)
+{
+ u32 val;
+ u32 prescale;
+
+ prescale = DIV_ROUND_UP(clk_get_rate(a3700_spi->clk), speed_hz);
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val = val & ~A3700_SPI_CLK_PRESCALE_MASK;
+
+ val = val | (prescale & A3700_SPI_CLK_PRESCALE_MASK);
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ if (prescale <= 2) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_TIME_REG);
+ val |= A3700_SPI_CLK_CAPT_EDGE;
+ spireg_write(a3700_spi, A3700_SPI_IF_TIME_REG, val);
+ }
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_CLK_POL | A3700_SPI_CLK_PHA);
+
+ if (mode & SPI_CPOL)
+ val |= A3700_SPI_CLK_POL;
+
+ if (mode & SPI_CPHA)
+ val |= A3700_SPI_CLK_PHA;
+
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
+static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (len == 4)
+ val |= A3700_SPI_BYTE_LEN;
+ else
+ val &= ~A3700_SPI_BYTE_LEN;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ a3700_spi->byte_len = len;
+}
+
+static int a3700_spi_init(struct a3700_spi *a3700_spi)
+{
+ struct spi_master *master = a3700_spi->master;
+ u32 val;
+ int i;
+
+ /* Reset SPI unit */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_SRST;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ for (i = 0; i < A3700_SPI_TIMEOUT; i++)
+ udelay(1);
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_SRST;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ /* Disable AUTO_CS and deactivate all chip-selects */
+ a3700_spi_auto_cs_unset(a3700_spi);
+ for (i = 0; i < master->num_chipselect; i++)
+ a3700_spi_deactivate_cs(a3700_spi, i);
+
+ a3700_spi_pin_mode_set(a3700_spi, 0);
+
+ /* Be sure that FIFO mode is disabled */
+ a3700_spi_fifo_mode_unset(a3700_spi);
+
+ /* Set SPI mode */
+ a3700_spi_mode_set(a3700_spi, master->mode_bits);
+
+ /* Reset counters */
+ spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG, 0);
+
+ /* Mask the interrupts and clear cause bits */
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
+
+ return 0;
+}
+
+static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
+{
+ struct spi_master *master = dev_id;
+ struct a3700_spi *a3700_spi;
+ u32 cause;
+
+ a3700_spi = spi_master_get_devdata(master);
+
+ /* Get interrupt causes */
+ cause = spireg_read(a3700_spi, A3700_SPI_INT_STAT_REG);
+
+ /* mask and acknowledge the SPI interrupts */
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, cause);
+
+ /* Wake up the transfer */
+ if (a3700_spi->wait_mask & cause)
+ complete(&a3700_spi->done);
+
+ return IRQ_HANDLED;
+}
+
+static bool a3700_spi_wait_completion(struct spi_device *spi)
+{
+ struct a3700_spi *a3700_spi;
+ unsigned int timeout;
+ unsigned int ctrl_reg;
+ unsigned long timeout_jiffies;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+
+ /* SPI interrupt is edge-triggered, which means an interrupt will
+ * be generated only when detecting a specific status bit changed
+ * from '0' to '1'. So when we start waiting for a interrupt, we
+ * need to check status bit in control reg first, if it is already 1,
+ * then we do not need to wait for interrupt
+ */
+ ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ if (a3700_spi->wait_mask & ctrl_reg)
+ return true;
+
+ reinit_completion(&a3700_spi->done);
+
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG,
+ a3700_spi->wait_mask);
+
+ timeout_jiffies = msecs_to_jiffies(A3700_SPI_TIMEOUT);
+ timeout = wait_for_completion_timeout(&a3700_spi->done,
+ timeout_jiffies);
+
+ a3700_spi->wait_mask = 0;
+
+ if (timeout)
+ return true;
+
+ /* there might be the case that right after we checked the
+ * status bits in this routine and before start to wait for
+ * interrupt by wait_for_completion_timeout, the interrupt
+ * happens, to avoid missing it we need to double check
+ * status bits in control reg, if it is already 1, then
+ * consider that we have the interrupt successfully and
+ * return true.
+ */
+ ctrl_reg = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ if (a3700_spi->wait_mask & ctrl_reg)
+ return true;
+
+ spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
+
+ return true;
+}
+
+static bool a3700_spi_transfer_wait(struct spi_device *spi,
+ unsigned int bit_mask)
+{
+ struct a3700_spi *a3700_spi;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+ a3700_spi->wait_mask = bit_mask;
+
+ return a3700_spi_wait_completion(spi);
+}
+
+static void a3700_spi_transfer_setup(struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi;
+
+ a3700_spi = spi_master_get_devdata(spi->master);
+
+ a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+}
+
+static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
+{
+ u32 val, data;
+
+ if (a3700_spi->buf_len % a3700_spi->byte_len)
+ return -EINVAL;
+
+ /* Read bytes from data in register */
+ val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+
+ if (a3700_spi->byte_len == 4)
+ data = be32_to_cpu(val);
+ else
+ data = val;
+
+ memcpy(a3700_spi->rx_buf, &data, a3700_spi->byte_len);
+
+ a3700_spi->buf_len -= a3700_spi->byte_len;
+ a3700_spi->rx_buf += a3700_spi->byte_len;
+
+ /* Request next 1 or 4 bytes data */
+ if (a3700_spi->buf_len)
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+ return 0;
+}
+
+static int a3700_spi_write_data(struct a3700_spi *a3700_spi)
+{
+ u32 val = 0;
+
+ if (a3700_spi->buf_len % a3700_spi->byte_len)
+ return -EINVAL;
+
+ /* Write bytes from data out register */
+ if (a3700_spi->byte_len == 4)
+ val = cpu_to_be32(*(u32 *)a3700_spi->tx_buf);
+ else
+ val = a3700_spi->tx_buf[0];
+
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+ a3700_spi->buf_len -= a3700_spi->byte_len;
+ a3700_spi->tx_buf += a3700_spi->byte_len;
+
+ return 0;
+}
+
+static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(spi->master);
+
+ if (!enable)
+ a3700_spi_activate_cs(a3700_spi, spi->chip_select);
+ else
+ a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
+}
+
+static int a3700_spi_prepare_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ struct spi_device *spi = message->spi;
+
+ a3700_spi_bytelen_set(a3700_spi, 1);
+
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static int a3700_spi_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret = 0;
+
+ a3700_spi_transfer_setup(spi, xfer);
+
+ a3700_spi->tx_buf = xfer->tx_buf;
+ a3700_spi->rx_buf = xfer->rx_buf;
+ a3700_spi->buf_len = xfer->len;
+
+ /* Start READ transfer by writing dummy data to DOUT register */
+ if (xfer->rx_buf)
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, 0);
+
+ while (a3700_spi->buf_len) {
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto err;
+ }
+
+ if (a3700_spi->tx_buf) {
+ ret = a3700_spi_write_data(a3700_spi);
+ if (ret)
+ goto err;
+ }
+
+ if (a3700_spi->rx_buf) {
+ ret = a3700_spi_read_data(a3700_spi);
+ if (ret)
+ goto err;
+ }
+ }
+
+err:
+ spi_finalize_current_transfer(master);
+ return ret;
+}
+
+static int a3700_spi_unprepare_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct spi_device *spi = message->spi;
+
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait transfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static const struct of_device_id a3700_spi_dt_ids[] = {
+ { .compatible = "marvell,armada-3700-spi", .data = NULL },
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, a3700_spi_dt_ids);
+
+static int a3700_spi_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *of_node = dev->of_node;
+ struct resource *res;
+ struct spi_master *master;
+ struct a3700_spi *spi;
+ u32 num_cs = 0;
+ int ret = 0;
+
+ master = spi_alloc_master(dev, sizeof(*spi));
+ if (!master) {
+ dev_err(dev, "master allocation failed\n");
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (of_property_read_u32(of_node, "num-cs", &num_cs)) {
+ dev_err(dev, "could not find num-cs\n");
+ ret = -ENXIO;
+ goto error;
+ }
+
+ master->bus_num = (pdev->id != -1) ? pdev->id : 0;
+ master->dev.of_node = of_node;
+ master->mode_bits = SPI_MODE_3;
+ master->num_chipselect = num_cs;
+ master->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(32);
+ master->prepare_message = a3700_spi_prepare_message;
+ master->transfer_one = a3700_spi_transfer_one;
+ master->unprepare_message = a3700_spi_unprepare_message;
+ master->set_cs = a3700_spi_set_cs;
+
+ platform_set_drvdata(pdev, master);
+
+ spi = spi_master_get_devdata(master);
+ memset(spi, 0, sizeof(struct a3700_spi));
+
+ spi->master = master;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ spi->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(spi->base)) {
+ ret = PTR_ERR(spi->base);
+ goto error;
+ }
+
+ spi->irq = platform_get_irq(pdev, 0);
+ if (spi->irq < 0) {
+ dev_err(dev, "could not get irq: %d\n", spi->irq);
+ ret = -ENXIO;
+ goto error;
+ }
+
+ init_completion(&spi->done);
+
+ spi->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(spi->clk)) {
+ dev_err(dev, "could not find clk: %ld\n", PTR_ERR(spi->clk));
+ goto error;
+ }
+
+ ret = clk_prepare_enable(spi->clk);
+ if (ret) {
+ dev_err(dev, "could not prepare clk: %d\n", ret);
+ goto error;
+ }
+
+ ret = a3700_spi_init(spi);
+ if (ret)
+ goto error_clk;
+
+ ret = devm_request_irq(dev, spi->irq, a3700_spi_interrupt, 0,
+ dev_name(dev), master);
+ if (ret) {
+ dev_err(dev, "could not request IRQ: %d\n", ret);
+ goto error_clk;
+ }
+
+ ret = devm_spi_register_master(dev, master);
+ if (ret) {
+ dev_err(dev, "Failed to register master\n");
+ goto error_clk;
+ }
+
+ dev_info(dev, "Marvell Armada 3700 SPI Controller at 0x%08lx, irq %d\n",
+ (unsigned long)res->start, spi->irq);
+
+ return 0;
+
+error_clk:
+ clk_disable_unprepare(spi->clk);
+error:
+ spi_master_put(master);
+out:
+ return ret;
+}
+
+static int a3700_spi_remove(struct platform_device *pdev)
+{
+ struct spi_master *master = platform_get_drvdata(pdev);
+ struct a3700_spi *spi = spi_master_get_devdata(master);
+
+ clk_disable_unprepare(spi->clk);
+ spi_master_put(master);
+
+ return 0;
+}
+
+static struct platform_driver a3700_spi_driver = {
+ .driver = {
+ .name = DRIVER_NAME,
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(a3700_spi_dt_ids),
+ },
+ .probe = a3700_spi_probe,
+ .remove = a3700_spi_remove,
+};
+
+module_platform_driver(a3700_spi_driver);
+
+MODULE_DESCRIPTION("Armada-3700 SPI driver");
+MODULE_AUTHOR("Wilson Ding <dingwei-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRIVER_NAME);
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v3 2/5] spi: armada-3700: Add support for the FIFO mode
From: Romain Perier @ 2016-12-01 10:27 UTC (permalink / raw)
To: Mark Brown, linux-spi-u79uwXL29TY76Z2rM5mHXA
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory Clement,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
Pawel Moll, Mark Rutland, Kumar Gala,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Thomas Petazzoni, Nadav Haklai, xigu-eYqpPyKDWXRBDgjK7y7TUQ,
dingwei-eYqpPyKDWXRBDgjK7y7TUQ
In-Reply-To: <20161201102719.4291-1-romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
In FIFO mode, dedicated registers are used to store the instruction,
the address, the read mode and the data. Write and Read FIFO are used
to store the outcoming or incoming data. The CPU no longer has to assert
each byte. The data FIFOs are accessible via DMA or by the CPU.
This commit adds support for the FIFO mode with the CPU.
Signed-off-by: Romain Perier <romain.perier-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Tested-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
Changes in v3:
- Don't enable the fifo mode based on the compatible string, we introduce
a module parameter "pio_mode". By default this option is set to zero, so
the fifo mode is enabled. Pass pio_mode=1 to the driver enables the PIO
mode.
- Added tag "Tested-by" by Gregory
Changes in v2:
- Removed a3700_spi_bytelen_set from the setup function, it was accidentally
let here and not required, as it is configured in the prepare callback now
(defaults to 4 for fifo mode). It solves unrecognized spi-nor flash memory
detection with jedec.
drivers/spi/spi-armada-3700.c | 414 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 404 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-armada-3700.c b/drivers/spi/spi-armada-3700.c
index 4115685..27a46cb 100644
--- a/drivers/spi/spi-armada-3700.c
+++ b/drivers/spi/spi-armada-3700.c
@@ -25,6 +25,11 @@
#include <linux/pinctrl/consumer.h>
#include <linux/spi/spi.h>
+static bool pio_mode;
+
+module_param(pio_mode, bool, 0);
+MODULE_PARM_DESC(pio_mode, "enable the PIO mode");
+
#define DRIVER_NAME "armada_3700_spi"
#define A3700_SPI_TIMEOUT 10
@@ -99,19 +104,28 @@
/* A3700_SPI_IF_TIME_REG */
#define A3700_SPI_CLK_CAPT_EDGE BIT(7)
+/* Flags and macros for struct a3700_spi */
+#define HAS_FIFO BIT(0)
+#define A3700_INSTR_CNT 1
+#define A3700_ADDR_CNT 3
+#define A3700_DUMMY_CNT 1
+
struct a3700_spi {
struct spi_master *master;
void __iomem *base;
struct clk *clk;
unsigned int irq;
unsigned int flags;
- bool last_xfer;
+ bool xmit_data;
const u8 *tx_buf;
u8 *rx_buf;
size_t buf_len;
u8 byte_len;
u32 wait_mask;
struct completion done;
+ u32 addr_cnt;
+ u32 instr_cnt;
+ size_t hdr_cnt;
};
static u32 spireg_read(struct a3700_spi *a3700_spi, u32 offset)
@@ -180,12 +194,15 @@ static int a3700_spi_pin_mode_set(struct a3700_spi *a3700_spi,
return 0;
}
-static void a3700_spi_fifo_mode_unset(struct a3700_spi *a3700_spi)
+static void a3700_spi_fifo_mode_set(struct a3700_spi *a3700_spi)
{
u32 val;
val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
- val &= ~A3700_SPI_FIFO_MODE;
+ if (a3700_spi->flags & HAS_FIFO)
+ val |= A3700_SPI_FIFO_MODE;
+ else
+ val &= ~A3700_SPI_FIFO_MODE;
spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
}
@@ -255,11 +272,30 @@ static void a3700_spi_bytelen_set(struct a3700_spi *a3700_spi, unsigned int len)
a3700_spi->byte_len = len;
}
+static int a3700_spi_fifo_flush(struct a3700_spi *a3700_spi)
+{
+ int timeout = A3700_SPI_TIMEOUT;
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_FIFO_FLUSH;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_FIFO_FLUSH))
+ return 0;
+ udelay(1);
+ }
+
+ return -ETIMEDOUT;
+}
+
static int a3700_spi_init(struct a3700_spi *a3700_spi)
{
struct spi_master *master = a3700_spi->master;
u32 val;
- int i;
+ int i, ret = 0;
/* Reset SPI unit */
val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
@@ -278,10 +314,8 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
for (i = 0; i < master->num_chipselect; i++)
a3700_spi_deactivate_cs(a3700_spi, i);
- a3700_spi_pin_mode_set(a3700_spi, 0);
-
- /* Be sure that FIFO mode is disabled */
- a3700_spi_fifo_mode_unset(a3700_spi);
+ /* Enable FIFO mode */
+ a3700_spi_fifo_mode_set(a3700_spi);
/* Set SPI mode */
a3700_spi_mode_set(a3700_spi, master->mode_bits);
@@ -294,7 +328,7 @@ static int a3700_spi_init(struct a3700_spi *a3700_spi)
spireg_write(a3700_spi, A3700_SPI_INT_MASK_REG, 0);
spireg_write(a3700_spi, A3700_SPI_INT_STAT_REG, ~0U);
- return 0;
+ return ret;
}
static irqreturn_t a3700_spi_interrupt(int irq, void *dev_id)
@@ -380,14 +414,34 @@ static bool a3700_spi_transfer_wait(struct spi_device *spi,
return a3700_spi_wait_completion(spi);
}
+static void a3700_spi_fifo_thres_set(struct a3700_spi *a3700_spi,
+ unsigned int bytes)
+{
+ u32 val;
+
+ if (a3700_spi->flags & HAS_FIFO) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_RFIFO_THRS_BIT);
+ val |= (bytes - 1) << A3700_SPI_RFIFO_THRS_BIT;
+ val &= ~(A3700_SPI_FIFO_THRS_MASK << A3700_SPI_WFIFO_THRS_BIT);
+ val |= (7 - bytes) << A3700_SPI_WFIFO_THRS_BIT;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ }
+}
+
static void a3700_spi_transfer_setup(struct spi_device *spi,
struct spi_transfer *xfer)
{
struct a3700_spi *a3700_spi;
+ unsigned int byte_len;
a3700_spi = spi_master_get_devdata(spi->master);
a3700_spi_clock_set(a3700_spi, xfer->speed_hz, spi->mode);
+
+ byte_len = xfer->bits_per_word >> 3;
+
+ a3700_spi_fifo_thres_set(a3700_spi, byte_len);
}
static int a3700_spi_read_data(struct a3700_spi *a3700_spi)
@@ -447,6 +501,168 @@ static void a3700_spi_set_cs(struct spi_device *spi, bool enable)
a3700_spi_deactivate_cs(a3700_spi, spi->chip_select);
}
+static void a3700_spi_header_set(struct a3700_spi *a3700_spi)
+{
+ u32 instr_cnt = 0, addr_cnt = 0, dummy_cnt = 0;
+ u32 val = 0;
+
+ /* Clear the header registers */
+ spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, 0);
+ spireg_write(a3700_spi, A3700_SPI_IF_RMODE_REG, 0);
+
+ /* Set header counters */
+ if (a3700_spi->tx_buf) {
+ if (a3700_spi->buf_len <= a3700_spi->instr_cnt) {
+ instr_cnt = a3700_spi->buf_len;
+ } else if (a3700_spi->buf_len <= (a3700_spi->instr_cnt +
+ a3700_spi->addr_cnt)) {
+ instr_cnt = a3700_spi->instr_cnt;
+ addr_cnt = a3700_spi->buf_len - instr_cnt;
+ } else if (a3700_spi->buf_len <= a3700_spi->hdr_cnt) {
+ instr_cnt = a3700_spi->instr_cnt;
+ addr_cnt = a3700_spi->addr_cnt;
+ /* Need to handle the normal write case with 1 byte
+ * data
+ */
+ if (!a3700_spi->tx_buf[instr_cnt + addr_cnt])
+ dummy_cnt = a3700_spi->buf_len - instr_cnt -
+ addr_cnt;
+ }
+ val |= ((instr_cnt & A3700_SPI_INSTR_CNT_MASK)
+ << A3700_SPI_INSTR_CNT_BIT);
+ val |= ((addr_cnt & A3700_SPI_ADDR_CNT_MASK)
+ << A3700_SPI_ADDR_CNT_BIT);
+ val |= ((dummy_cnt & A3700_SPI_DUMMY_CNT_MASK)
+ << A3700_SPI_DUMMY_CNT_BIT);
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_HDR_CNT_REG, val);
+
+ /* Update the buffer length to be transferred */
+ a3700_spi->buf_len -= (instr_cnt + addr_cnt + dummy_cnt);
+
+ /* Set Instruction */
+ val = 0;
+ while (instr_cnt--) {
+ val = (val << 8) | a3700_spi->tx_buf[0];
+ a3700_spi->tx_buf++;
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_INST_REG, val);
+
+ /* Set Address */
+ val = 0;
+ while (addr_cnt--) {
+ val = (val << 8) | a3700_spi->tx_buf[0];
+ a3700_spi->tx_buf++;
+ }
+ spireg_write(a3700_spi, A3700_SPI_IF_ADDR_REG, val);
+}
+
+static int a3700_is_wfifo_full(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+ return (val & A3700_SPI_WFIFO_FULL);
+}
+
+static int a3700_spi_fifo_write(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+ int i = 0;
+
+ while (!a3700_is_wfifo_full(a3700_spi) && a3700_spi->buf_len) {
+ val = 0;
+ if (a3700_spi->buf_len >= 4) {
+ val = cpu_to_le32(*(u32 *)a3700_spi->tx_buf);
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG, val);
+
+ a3700_spi->buf_len -= 4;
+ a3700_spi->tx_buf += 4;
+ } else {
+ /*
+ * If the remained buffer length is less than 4-bytes,
+ * we should pad the write buffer with all ones. So that
+ * it avoids overwrite the unexpected bytes following
+ * the last one.
+ */
+ val = GENMASK(31, 0);
+ while (a3700_spi->buf_len) {
+ val &= ~(0xff << (8 * i));
+ val |= *a3700_spi->tx_buf++ << (8 * i);
+ i++;
+ a3700_spi->buf_len--;
+
+ spireg_write(a3700_spi, A3700_SPI_DATA_OUT_REG,
+ val);
+ }
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static int a3700_is_rfifo_empty(struct a3700_spi *a3700_spi)
+{
+ u32 val = spireg_read(a3700_spi, A3700_SPI_IF_CTRL_REG);
+
+ return (val & A3700_SPI_RFIFO_EMPTY);
+}
+
+static int a3700_spi_fifo_read(struct a3700_spi *a3700_spi)
+{
+ u32 val;
+
+ while (!a3700_is_rfifo_empty(a3700_spi) && a3700_spi->buf_len) {
+ val = spireg_read(a3700_spi, A3700_SPI_DATA_IN_REG);
+ if (a3700_spi->buf_len >= 4) {
+ u32 data = le32_to_cpu(val);
+ memcpy(a3700_spi->rx_buf, &data, 4);
+
+ a3700_spi->buf_len -= 4;
+ a3700_spi->rx_buf += 4;
+ } else {
+ /*
+ * When remain bytes is not larger than 4, we should
+ * avoid memory overwriting and just write the left rx
+ * buffer bytes.
+ */
+ while (a3700_spi->buf_len) {
+ *a3700_spi->rx_buf = val & 0xff;
+ val >>= 8;
+
+ a3700_spi->buf_len--;
+ a3700_spi->rx_buf++;
+ }
+ }
+ }
+
+ return 0;
+}
+
+static void a3700_spi_transfer_abort_fifo(struct a3700_spi *a3700_spi)
+{
+ int timeout = A3700_SPI_TIMEOUT;
+ u32 val;
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_XFER_START))
+ break;
+ udelay(1);
+ }
+
+ a3700_spi_fifo_flush(a3700_spi);
+
+ val &= ~A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+}
+
static int a3700_spi_prepare_message(struct spi_master *master,
struct spi_message *message)
{
@@ -463,12 +679,28 @@ static int a3700_spi_prepare_message(struct spi_master *master,
return 0;
}
+static int a3700_spi_prepare_fifo_message(struct spi_master *master,
+ struct spi_message *message)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret;
+
+ /* Flush the FIFOs */
+ ret = a3700_spi_fifo_flush(a3700_spi);
+ if (ret)
+ return ret;
+
+ a3700_spi_bytelen_set(a3700_spi, 4);
+
+ return 0;
+}
+
static int a3700_spi_transfer_one(struct spi_master *master,
struct spi_device *spi,
struct spi_transfer *xfer)
{
struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
- int ret = 0;
+ int ret;
a3700_spi_transfer_setup(spi, xfer);
@@ -505,6 +737,151 @@ static int a3700_spi_transfer_one(struct spi_master *master,
return ret;
}
+static int a3700_spi_fifo_transfer_one(struct spi_master *master,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct a3700_spi *a3700_spi = spi_master_get_devdata(master);
+ int ret = 0, timeout = A3700_SPI_TIMEOUT;
+ unsigned int nbits = 0;
+ u32 val;
+
+ a3700_spi_transfer_setup(spi, xfer);
+
+ a3700_spi->tx_buf = xfer->tx_buf;
+ a3700_spi->rx_buf = xfer->rx_buf;
+ a3700_spi->buf_len = xfer->len;
+
+ /* SPI transfer headers */
+ a3700_spi_header_set(a3700_spi);
+
+ if (xfer->tx_buf)
+ nbits = xfer->tx_nbits;
+ else if (xfer->rx_buf)
+ nbits = xfer->rx_nbits;
+
+ a3700_spi_pin_mode_set(a3700_spi, nbits);
+
+ if (xfer->rx_buf) {
+ /* Set read data length */
+ spireg_write(a3700_spi, A3700_SPI_IF_DIN_CNT_REG,
+ a3700_spi->buf_len);
+ /* Start READ transfer */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val &= ~A3700_SPI_RW_EN;
+ val |= A3700_SPI_XFER_START;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ } else if (xfer->tx_buf) {
+ /* Start Write transfer */
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= (A3700_SPI_XFER_START | A3700_SPI_RW_EN);
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+
+ /*
+ * If there are data to be written to the SPI device, xmit_data
+ * flag is set true; otherwise the instruction in SPI_INSTR does
+ * not require data to be written to the SPI device, then
+ * xmit_data flag is set false.
+ */
+ a3700_spi->xmit_data = (a3700_spi->buf_len != 0);
+ }
+
+ while (a3700_spi->buf_len) {
+ if (a3700_spi->tx_buf) {
+ /* Wait wfifo ready */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_WFIFO_RDY)) {
+ dev_err(&spi->dev,
+ "wait wfifo ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+ /* Fill up the wfifo */
+ ret = a3700_spi_fifo_write(a3700_spi);
+ if (ret)
+ goto error;
+ } else if (a3700_spi->rx_buf) {
+ /* Wait rfifo ready */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_RFIFO_RDY)) {
+ dev_err(&spi->dev,
+ "wait rfifo ready timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+ /* Drain out the rfifo */
+ ret = a3700_spi_fifo_read(a3700_spi);
+ if (ret)
+ goto error;
+ }
+ }
+
+ /*
+ * Stop a write transfer in fifo mode:
+ * - wait all the bytes in wfifo to be shifted out
+ * - set XFER_STOP bit
+ * - wait XFER_START bit clear
+ * - clear XFER_STOP bit
+ * Stop a read transfer in fifo mode:
+ * - the hardware is to reset the XFER_START bit
+ * after the number of bytes indicated in DIN_CNT
+ * register
+ * - just wait XFER_START bit clear
+ */
+ if (a3700_spi->tx_buf) {
+ if (a3700_spi->xmit_data) {
+ /*
+ * If there are data written to the SPI device, wait
+ * until SPI_WFIFO_EMPTY is 1 to wait for all data to
+ * transfer out of write FIFO.
+ */
+ if (!a3700_spi_transfer_wait(spi,
+ A3700_SPI_WFIFO_EMPTY)) {
+ dev_err(&spi->dev, "wait wfifo empty timed out\n");
+ return -ETIMEDOUT;
+ }
+ } else {
+ /*
+ * If the instruction in SPI_INSTR does not require data
+ * to be written to the SPI device, wait until SPI_RDY
+ * is 1 for the SPI interface to be in idle.
+ */
+ if (!a3700_spi_transfer_wait(spi, A3700_SPI_XFER_RDY)) {
+ dev_err(&spi->dev, "wait xfer ready timed out\n");
+ return -ETIMEDOUT;
+ }
+ }
+
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ val |= A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ }
+
+ while (--timeout) {
+ val = spireg_read(a3700_spi, A3700_SPI_IF_CFG_REG);
+ if (!(val & A3700_SPI_XFER_START))
+ break;
+ udelay(1);
+ }
+
+ if (timeout == 0) {
+ dev_err(&spi->dev, "wait transfer start clear timed out\n");
+ ret = -ETIMEDOUT;
+ goto error;
+ }
+
+ val &= ~A3700_SPI_XFER_STOP;
+ spireg_write(a3700_spi, A3700_SPI_IF_CFG_REG, val);
+ goto out;
+
+error:
+ a3700_spi_transfer_abort_fifo(a3700_spi);
+out:
+ spi_finalize_current_transfer(master);
+
+ return ret;
+}
+
static int a3700_spi_unprepare_message(struct spi_master *master,
struct spi_message *message)
{
@@ -593,6 +970,23 @@ static int a3700_spi_probe(struct platform_device *pdev)
goto error;
}
+ if (!pio_mode) {
+ master->prepare_message = a3700_spi_prepare_fifo_message;
+ master->transfer_one = a3700_spi_fifo_transfer_one;
+
+ spi->flags |= HAS_FIFO;
+ spi->instr_cnt = A3700_INSTR_CNT;
+ spi->addr_cnt = A3700_ADDR_CNT;
+ spi->hdr_cnt = A3700_INSTR_CNT + A3700_ADDR_CNT +
+ A3700_DUMMY_CNT;
+ master->mode_bits |= (SPI_RX_DUAL | SPI_RX_DUAL |
+ SPI_RX_QUAD | SPI_TX_QUAD);
+ } else {
+ master->prepare_message = a3700_spi_prepare_message;
+ master->transfer_one = a3700_spi_transfer_one;
+ master->unprepare_message = a3700_spi_unprepare_message;
+ }
+
ret = a3700_spi_init(spi);
if (ret)
goto error_clk;
--
2.9.3
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox