* Re: [PATCH v8 3/3] phy: intel: Add driver support for ComboPhy
From: Dilip Kota @ 2020-05-15 8:05 UTC (permalink / raw)
To: Vinod Koul
Cc: linux-kernel, kishon, devicetree, robh, andriy.shevchenko,
cheol.yong.kim, chuanhua.lei, qi-ming.wu, yixin.zhu
In-Reply-To: <20200515073959.GM333670@vkoul-mobl>
On 5/15/2020 3:39 PM, Vinod Koul wrote:
> On 15-05-20, 12:30, Dilip Kota wrote:
>> ComboPhy subsystem provides PHYs for various
>> controllers like PCIe, SATA and EMAC.
>>
>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>> ---
>> Changes on v8:
>> As per PHY Maintainer's request add description for doing register access
>> through regmap in comments.
> I dont see any comments below, and my script tells me this is *same* as
> v7, did you send the wrong version?
My bad. I missed the git add step and sent patch series.
Will resend the patch series.
Regards,
Dilip
>
>> Changes on v7:
>> Use device_node_to_regmap instead of fwnode_to_regmap
>>
>> Changes on v6:
>> No changes
>>
>> Changes on v5:
>> Add changes as per inputs from Andy and Rob:
>> DT node uses phy-mode values as defined in "include/dt-bindings/phy/phy.h",
>> add changes to handle it.
>> ComboPhy no longer has children nodes, and children node properties(reset)
>> moved to parent node, so do the code changes accordingly.
>> Add _xlate() function to pass the appropriate phy handle.
>> Fix couple of nitpicks.
>>
>> Changes on v4:
>> Address review comments
>> Remove dependency on OF config
>> Update copyright to 2019-2020
>> Define register macro PAD_DIS_CFG instead of const variable inside function.
>> Improve the error prints, and error returns.
>> Call put_device(dev), for get_dev_from_fwnode()
>> Move platform_set_drvdata() at the end of the probe().
>> Correct alignment in phy_ops intel_cbphy_ops.
>> Correct commented lines with proper vocabulary and punctuation.
>> Add/remove commas for the required constant arrays and enums.
>> Remove in driver:
>> linux/kernel.h, not required
>> macros: PCIE_PHY_MPLLA_CTRL, PCIE_PHY_MPLLB_CTRL
>> temp variable u32 prop;
>> Change function names:
>> intel_cbphy_iphy_dt_parse() -> intel_cbphy_iphy_fwnode_parse()
>> intel_cbphy_dt_sanity_check() -> intel_cbphy_sanity_check()
>> intel_cbphy_dt_parse() -> intel_cbphy_fwnode_parse()
>>
>> Changes on v3:
>> Remove intel_iphy_names
>> Remove struct phy in struct intel_cbphy_iphy
>> Imporve if conditions logic
>> Use fwnode_to_regmap()
>> Call devm_of_platform_populate() to populate child nodes
>> Fix reset sequence during phy_init
>> Add SoC specific compatible "intel,combophy-lgm"
>> Add description for enums
>> Remove default case in switch {} intel_cbphy_set_mode() as it
>> never happens.
>> Use mutex_lock to synchronise combophy initialization across
>> two phys.
>> Change init_cnt to u32 datatype as it is within mutex lock.
>> Correct error handling of
>> fwnode_property_read_u32_array(fwnode, "intel,phy-mode", ...)
>>
>> drivers/phy/intel/Kconfig | 14 +
>> drivers/phy/intel/Makefile | 1 +
>> drivers/phy/intel/phy-intel-combo.c | 627 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 642 insertions(+)
>> create mode 100644 drivers/phy/intel/phy-intel-combo.c
>>
>> diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig
>> index 4ea6a8897cd7..3b40eb7b4fb4 100644
>> --- a/drivers/phy/intel/Kconfig
>> +++ b/drivers/phy/intel/Kconfig
>> @@ -2,6 +2,20 @@
>> #
>> # Phy drivers for Intel Lightning Mountain(LGM) platform
>> #
>> +config PHY_INTEL_COMBO
>> + bool "Intel ComboPHY driver"
>> + depends on X86 || COMPILE_TEST
>> + depends on OF && HAS_IOMEM
>> + select MFD_SYSCON
>> + select GENERIC_PHY
>> + select REGMAP
>> + help
>> + Enable this to support Intel ComboPhy.
>> +
>> + This driver configures ComboPhy subsystem on Intel gateway
>> + chipsets which provides PHYs for various controllers, EMAC,
>> + SATA and PCIe.
>> +
>> config PHY_INTEL_EMMC
>> tristate "Intel EMMC PHY driver"
>> select GENERIC_PHY
>> diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile
>> index 6b876a75599d..233d530dadde 100644
>> --- a/drivers/phy/intel/Makefile
>> +++ b/drivers/phy/intel/Makefile
>> @@ -1,2 +1,3 @@
>> # SPDX-License-Identifier: GPL-2.0
>> +obj-$(CONFIG_PHY_INTEL_COMBO) += phy-intel-combo.o
>> obj-$(CONFIG_PHY_INTEL_EMMC) += phy-intel-emmc.o
>> diff --git a/drivers/phy/intel/phy-intel-combo.c b/drivers/phy/intel/phy-intel-combo.c
>> new file mode 100644
>> index 000000000000..04ad595e21e4
>> --- /dev/null
>> +++ b/drivers/phy/intel/phy-intel-combo.c
>> @@ -0,0 +1,627 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Intel Combo-PHY driver
>> + *
>> + * Copyright (C) 2019-2020 Intel Corporation.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +
>> +#include <dt-bindings/phy/phy.h>
>> +
>> +#define PCIE_PHY_GEN_CTRL 0x00
>> +#define PCIE_PHY_CLK_PAD BIT(17)
>> +
>> +#define PAD_DIS_CFG 0x174
>> +
>> +#define PCS_XF_ATE_OVRD_IN_2 0x3008
>> +#define ADAPT_REQ_MSK GENMASK(5, 4)
>> +
>> +#define PCS_XF_RX_ADAPT_ACK 0x3010
>> +#define RX_ADAPT_ACK_BIT BIT(0)
>> +
>> +#define CR_ADDR(addr, lane) (((addr) + (lane) * 0x100) << 2)
>> +#define REG_COMBO_MODE(x) ((x) * 0x200)
>> +#define REG_CLK_DISABLE(x) ((x) * 0x200 + 0x124)
>> +
>> +#define COMBO_PHY_ID(x) ((x)->parent->id)
>> +#define PHY_ID(x) ((x)->id)
>> +
>> +#define CLK_100MHZ 100000000
>> +#define CLK_156_25MHZ 156250000
>> +
>> +static const unsigned long intel_iphy_clk_rates[] = {
>> + CLK_100MHZ, CLK_156_25MHZ, CLK_100MHZ,
>> +};
>> +
>> +enum {
>> + PHY_0,
>> + PHY_1,
>> + PHY_MAX_NUM
>> +};
>> +
>> +/*
>> + * Clock Register bit fields to enable clocks
>> + * for ComboPhy according to the mode.
>> + */
>> +enum intel_phy_mode {
>> + PHY_PCIE_MODE = 0,
>> + PHY_XPCS_MODE,
>> + PHY_SATA_MODE,
>> +};
>> +
>> +/* ComboPhy mode Register values */
>> +enum intel_combo_mode {
>> + PCIE0_PCIE1_MODE = 0,
>> + PCIE_DL_MODE,
>> + RXAUI_MODE,
>> + XPCS0_XPCS1_MODE,
>> + SATA0_SATA1_MODE,
>> +};
>> +
>> +enum aggregated_mode {
>> + PHY_SL_MODE,
>> + PHY_DL_MODE,
>> +};
>> +
>> +struct intel_combo_phy;
>> +
>> +struct intel_cbphy_iphy {
>> + struct phy *phy;
>> + struct intel_combo_phy *parent;
>> + struct reset_control *app_rst;
>> + u32 id;
>> +};
>> +
>> +struct intel_combo_phy {
>> + struct device *dev;
>> + struct clk *core_clk;
>> + unsigned long clk_rate;
>> + void __iomem *app_base;
>> + void __iomem *cr_base;
>> + struct regmap *syscfg;
>> + struct regmap *hsiocfg;
>> + u32 id;
>> + u32 bid;
>> + struct reset_control *phy_rst;
>> + struct reset_control *core_rst;
>> + struct intel_cbphy_iphy iphy[PHY_MAX_NUM];
>> + enum intel_phy_mode phy_mode;
>> + enum aggregated_mode aggr_mode;
>> + u32 init_cnt;
>> + struct mutex lock;
>> +};
>> +
>> +static int intel_cbphy_iphy_enable(struct intel_cbphy_iphy *iphy, bool set)
>> +{
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + u32 mask = BIT(cbphy->phy_mode * 2 + iphy->id);
>> + u32 val;
>> +
>> + /* Register: 0 is enable, 1 is disable */
>> + val = set ? 0 : mask;
>> +
>> + return regmap_update_bits(cbphy->hsiocfg, REG_CLK_DISABLE(cbphy->bid),
>> + mask, val);
>> +}
>> +
>> +static int intel_cbphy_pcie_refclk_cfg(struct intel_cbphy_iphy *iphy, bool set)
>> +{
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + u32 mask = BIT(cbphy->id * 2 + iphy->id);
>> + u32 val;
>> +
>> + /* Register: 0 is enable, 1 is disable */
>> + val = set ? 0 : mask;
>> +
>> + return regmap_update_bits(cbphy->syscfg, PAD_DIS_CFG, mask, val);
>> +}
>> +
>> +static inline void combo_phy_w32_off_mask(void __iomem *base, unsigned int reg,
>> + u32 mask, u32 val)
>> +{
>> + u32 reg_val;
>> +
>> + reg_val = readl(base + reg);
>> + reg_val &= ~mask;
>> + reg_val |= FIELD_PREP(mask, val);
>> + writel(reg_val, base + reg);
>> +}
>> +
>> +static int intel_cbphy_iphy_cfg(struct intel_cbphy_iphy *iphy,
>> + int (*phy_cfg)(struct intel_cbphy_iphy *))
>> +{
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + int ret;
>> +
>> + ret = phy_cfg(iphy);
>> + if (ret)
>> + return ret;
>> +
>> + if (cbphy->aggr_mode != PHY_DL_MODE)
>> + return 0;
>> +
>> + return phy_cfg(&cbphy->iphy[PHY_1]);
>> +}
>> +
>> +static int intel_cbphy_pcie_en_pad_refclk(struct intel_cbphy_iphy *iphy)
>> +{
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + int ret;
>> +
>> + ret = intel_cbphy_pcie_refclk_cfg(iphy, true);
>> + if (ret) {
>> + dev_err(cbphy->dev, "Failed to enable PCIe pad refclk\n");
>> + return ret;
>> + }
>> +
>> + if (cbphy->init_cnt)
>> + return 0;
>> +
>> + combo_phy_w32_off_mask(cbphy->app_base, PCIE_PHY_GEN_CTRL,
>> + PCIE_PHY_CLK_PAD, 0);
>> +
>> + /* Delay for stable clock PLL */
>> + usleep_range(50, 100);
>> +
>> + return 0;
>> +}
>> +
>> +static int intel_cbphy_pcie_dis_pad_refclk(struct intel_cbphy_iphy *iphy)
>> +{
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + int ret;
>> +
>> + ret = intel_cbphy_pcie_refclk_cfg(iphy, false);
>> + if (ret) {
>> + dev_err(cbphy->dev, "Failed to disable PCIe pad refclk\n");
>> + return ret;
>> + }
>> +
>> + if (cbphy->init_cnt)
>> + return 0;
>> +
>> + combo_phy_w32_off_mask(cbphy->app_base, PCIE_PHY_GEN_CTRL,
>> + PCIE_PHY_CLK_PAD, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static int intel_cbphy_set_mode(struct intel_combo_phy *cbphy)
>> +{
>> + enum aggregated_mode aggr = cbphy->aggr_mode;
>> + struct device *dev = cbphy->dev;
>> + enum intel_combo_mode cb_mode;
>> + enum intel_phy_mode mode;
>> + int ret;
>> +
>> + mode = cbphy->phy_mode;
>> +
>> + switch (mode) {
>> + case PHY_PCIE_MODE:
>> + cb_mode = (aggr == PHY_DL_MODE) ? PCIE_DL_MODE : PCIE0_PCIE1_MODE;
>> + break;
>> +
>> + case PHY_XPCS_MODE:
>> + cb_mode = (aggr == PHY_DL_MODE) ? RXAUI_MODE : XPCS0_XPCS1_MODE;
>> + break;
>> +
>> + case PHY_SATA_MODE:
>> + if (aggr == PHY_DL_MODE) {
>> + dev_err(dev, "Mode:%u not support dual lane!\n", mode);
>> + return -EINVAL;
>> + }
>> +
>> + cb_mode = SATA0_SATA1_MODE;
>> + break;
>> + }
>> +
>> + ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode);
>> + if (ret)
>> + dev_err(dev, "Failed to set ComboPhy mode: %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static void intel_cbphy_rst_assert(struct intel_combo_phy *cbphy)
>> +{
>> + reset_control_assert(cbphy->core_rst);
>> + reset_control_assert(cbphy->phy_rst);
>> +}
>> +
>> +static void intel_cbphy_rst_deassert(struct intel_combo_phy *cbphy)
>> +{
>> + reset_control_deassert(cbphy->core_rst);
>> + reset_control_deassert(cbphy->phy_rst);
>> + /* Delay to ensure reset process is done */
>> + usleep_range(10, 20);
>> +}
>> +
>> +static int intel_cbphy_iphy_power_on(struct intel_cbphy_iphy *iphy)
>> +{
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + int ret;
>> +
>> + if (!cbphy->init_cnt) {
>> + ret = clk_prepare_enable(cbphy->core_clk);
>> + if (ret) {
>> + dev_err(cbphy->dev, "Clock enable failed!\n");
>> + return ret;
>> + }
>> +
>> + ret = clk_set_rate(cbphy->core_clk, cbphy->clk_rate);
>> + if (ret) {
>> + dev_err(cbphy->dev, "Clock freq set to %lu failed!\n",
>> + cbphy->clk_rate);
>> + goto clk_err;
>> + }
>> +
>> + intel_cbphy_rst_assert(cbphy);
>> + intel_cbphy_rst_deassert(cbphy);
>> + ret = intel_cbphy_set_mode(cbphy);
>> + if (ret)
>> + goto clk_err;
>> + }
>> +
>> + ret = intel_cbphy_iphy_enable(iphy, true);
>> + if (ret) {
>> + dev_err(cbphy->dev, "Failed enabling PHY core\n");
>> + goto clk_err;
>> + }
>> +
>> + ret = reset_control_deassert(iphy->app_rst);
>> + if (ret) {
>> + dev_err(cbphy->dev, "PHY(%u:%u) reset deassert failed!\n",
>> + COMBO_PHY_ID(iphy), PHY_ID(iphy));
>> + goto clk_err;
>> + }
>> +
>> + /* Delay to ensure reset process is done */
>> + udelay(1);
>> +
>> + return 0;
>> +
>> +clk_err:
>> + clk_disable_unprepare(cbphy->core_clk);
>> +
>> + return ret;
>> +}
>> +
>> +static int intel_cbphy_iphy_power_off(struct intel_cbphy_iphy *iphy)
>> +{
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + int ret;
>> +
>> + ret = reset_control_assert(iphy->app_rst);
>> + if (ret) {
>> + dev_err(cbphy->dev, "PHY(%u:%u) reset assert failed!\n",
>> + COMBO_PHY_ID(iphy), PHY_ID(iphy));
>> + return ret;
>> + }
>> +
>> + ret = intel_cbphy_iphy_enable(iphy, false);
>> + if (ret) {
>> + dev_err(cbphy->dev, "Failed disabling PHY core\n");
>> + return ret;
>> + }
>> +
>> + if (cbphy->init_cnt)
>> + return 0;
>> +
>> + clk_disable_unprepare(cbphy->core_clk);
>> + intel_cbphy_rst_assert(cbphy);
>> +
>> + return 0;
>> +}
>> +
>> +static int intel_cbphy_init(struct phy *phy)
>> +{
>> + struct intel_cbphy_iphy *iphy = phy_get_drvdata(phy);
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + int ret;
>> +
>> + mutex_lock(&cbphy->lock);
>> + ret = intel_cbphy_iphy_cfg(iphy, intel_cbphy_iphy_power_on);
>> + if (ret)
>> + goto err;
>> +
>> + if (cbphy->phy_mode == PHY_PCIE_MODE) {
>> + ret = intel_cbphy_iphy_cfg(iphy, intel_cbphy_pcie_en_pad_refclk);
>> + if (ret)
>> + goto err;
>> + }
>> +
>> + cbphy->init_cnt++;
>> +
>> +err:
>> + mutex_unlock(&cbphy->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static int intel_cbphy_exit(struct phy *phy)
>> +{
>> + struct intel_cbphy_iphy *iphy = phy_get_drvdata(phy);
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + int ret;
>> +
>> + mutex_lock(&cbphy->lock);
>> + cbphy->init_cnt--;
>> + if (cbphy->phy_mode == PHY_PCIE_MODE) {
>> + ret = intel_cbphy_iphy_cfg(iphy, intel_cbphy_pcie_dis_pad_refclk);
>> + if (ret)
>> + goto err;
>> + }
>> +
>> + ret = intel_cbphy_iphy_cfg(iphy, intel_cbphy_iphy_power_off);
>> +
>> +err:
>> + mutex_unlock(&cbphy->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static int intel_cbphy_calibrate(struct phy *phy)
>> +{
>> + struct intel_cbphy_iphy *iphy = phy_get_drvdata(phy);
>> + struct intel_combo_phy *cbphy = iphy->parent;
>> + void __iomem *cr_base = cbphy->cr_base;
>> + int val, ret, id;
>> +
>> + if (cbphy->phy_mode != PHY_XPCS_MODE)
>> + return 0;
>> +
>> + id = PHY_ID(iphy);
>> +
>> + /* trigger auto RX adaptation */
>> + combo_phy_w32_off_mask(cr_base, CR_ADDR(PCS_XF_ATE_OVRD_IN_2, id),
>> + ADAPT_REQ_MSK, 3);
>> + /* Wait RX adaptation to finish */
>> + ret = readl_poll_timeout(cr_base + CR_ADDR(PCS_XF_RX_ADAPT_ACK, id),
>> + val, val & RX_ADAPT_ACK_BIT, 10, 5000);
>> + if (ret)
>> + dev_err(cbphy->dev, "RX Adaptation failed!\n");
>> + else
>> + dev_dbg(cbphy->dev, "RX Adaptation success!\n");
>> +
>> + /* Stop RX adaptation */
>> + combo_phy_w32_off_mask(cr_base, CR_ADDR(PCS_XF_ATE_OVRD_IN_2, id),
>> + ADAPT_REQ_MSK, 0);
>> +
>> + return ret;
>> +}
>> +
>> +static int intel_cbphy_fwnode_parse(struct intel_combo_phy *cbphy)
>> +{
>> + struct device *dev = cbphy->dev;
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct fwnode_handle *fwnode = dev_fwnode(dev);
>> + struct fwnode_reference_args ref;
>> + int ret;
>> + u32 val;
>> +
>> + cbphy->core_clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(cbphy->core_clk)) {
>> + ret = PTR_ERR(cbphy->core_clk);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "Get clk failed:%d!\n", ret);
>> + return ret;
>> + }
>> +
>> + cbphy->core_rst = devm_reset_control_get_optional(dev, "core");
>> + if (IS_ERR(cbphy->core_rst)) {
>> + ret = PTR_ERR(cbphy->core_rst);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "Get core reset control err: %d!\n", ret);
>> + return ret;
>> + }
>> +
>> + cbphy->phy_rst = devm_reset_control_get_optional(dev, "phy");
>> + if (IS_ERR(cbphy->phy_rst)) {
>> + ret = PTR_ERR(cbphy->phy_rst);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "Get PHY reset control err: %d!\n", ret);
>> + return ret;
>> + }
>> +
>> + cbphy->iphy[0].app_rst = devm_reset_control_get_optional(dev, "iphy0");
>> + if (IS_ERR(cbphy->iphy[0].app_rst)) {
>> + ret = PTR_ERR(cbphy->iphy[0].app_rst);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "Get phy0 reset control err: %d!\n", ret);
>> + return ret;
>> + }
>> +
>> + cbphy->iphy[1].app_rst = devm_reset_control_get_optional(dev, "iphy1");
>> + if (IS_ERR(cbphy->iphy[1].app_rst)) {
>> + ret = PTR_ERR(cbphy->iphy[1].app_rst);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "Get phy1 reset control err: %d!\n", ret);
>> + return ret;
>> + }
>> +
>> + cbphy->app_base = devm_platform_ioremap_resource_byname(pdev, "app");
>> + if (IS_ERR(cbphy->app_base))
>> + return PTR_ERR(cbphy->app_base);
>> +
>> + cbphy->cr_base = devm_platform_ioremap_resource_byname(pdev, "core");
>> + if (IS_ERR(cbphy->cr_base))
>> + return PTR_ERR(cbphy->cr_base);
>> +
>> + ret = fwnode_property_get_reference_args(fwnode, "intel,syscfg", NULL,
>> + 1, 0, &ref);
>> + if (ret < 0)
>> + return ret;
>> +
>> + cbphy->id = ref.args[0];
>> + cbphy->syscfg = device_node_to_regmap(to_of_node(ref.fwnode));
>> + fwnode_handle_put(ref.fwnode);
>> +
>> + ret = fwnode_property_get_reference_args(fwnode, "intel,hsio", NULL, 1,
>> + 0, &ref);
>> + if (ret < 0)
>> + return ret;
>> +
>> + cbphy->bid = ref.args[0];
>> + cbphy->hsiocfg = device_node_to_regmap(to_of_node(ref.fwnode));
>> + fwnode_handle_put(ref.fwnode);
>> +
>> + ret = fwnode_property_read_u32_array(fwnode, "intel,phy-mode", &val, 1);
>> + if (ret)
>> + return ret;
>> +
>> + switch (val) {
>> + case PHY_TYPE_PCIE:
>> + cbphy->phy_mode = PHY_PCIE_MODE;
>> + break;
>> +
>> + case PHY_TYPE_SATA:
>> + cbphy->phy_mode = PHY_SATA_MODE;
>> + break;
>> +
>> + case PHY_TYPE_XPCS:
>> + cbphy->phy_mode = PHY_XPCS_MODE;
>> + break;
>> +
>> + default:
>> + dev_err(dev, "Invalid PHY mode: %u\n", val);
>> + return -EINVAL;
>> + }
>> +
>> + cbphy->clk_rate = intel_iphy_clk_rates[cbphy->phy_mode];
>> +
>> + if (fwnode_property_present(fwnode, "intel,aggregation"))
>> + cbphy->aggr_mode = PHY_DL_MODE;
>> + else
>> + cbphy->aggr_mode = PHY_SL_MODE;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct phy_ops intel_cbphy_ops = {
>> + .init = intel_cbphy_init,
>> + .exit = intel_cbphy_exit,
>> + .calibrate = intel_cbphy_calibrate,
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static struct phy *intel_cbphy_xlate(struct device *dev,
>> + struct of_phandle_args *args)
>> +{
>> + struct intel_combo_phy *cbphy = dev_get_drvdata(dev);
>> + u32 iphy_id;
>> +
>> + if (args->args_count < 1) {
>> + dev_err(dev, "Invalid number of arguments\n");
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + iphy_id = args->args[0];
>> + if (iphy_id >= PHY_MAX_NUM) {
>> + dev_err(dev, "Invalid phy instance %d\n", iphy_id);
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + if (cbphy->aggr_mode == PHY_DL_MODE && iphy_id == PHY_1) {
>> + dev_err(dev, "Invalid. ComboPhy is in Dual lane mode %d\n", iphy_id);
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + return cbphy->iphy[iphy_id].phy;
>> +}
>> +
>> +static int intel_cbphy_create(struct intel_combo_phy *cbphy)
>> +{
>> + struct phy_provider *phy_provider;
>> + struct device *dev = cbphy->dev;
>> + struct intel_cbphy_iphy *iphy;
>> + int i;
>> +
>> + for (i = 0; i < PHY_MAX_NUM; i++) {
>> + iphy = &cbphy->iphy[i];
>> + iphy->parent = cbphy;
>> + iphy->id = i;
>> +
>> + /* In dual lane mode skip phy creation for the second phy */
>> + if (cbphy->aggr_mode == PHY_DL_MODE && iphy->id == PHY_1)
>> + continue;
>> +
>> + iphy->phy = devm_phy_create(dev, NULL, &intel_cbphy_ops);
>> + if (IS_ERR(iphy->phy)) {
>> + dev_err(dev, "PHY[%u:%u]: create PHY instance failed!\n",
>> + COMBO_PHY_ID(iphy), PHY_ID(iphy));
>> +
>> + return PTR_ERR(iphy->phy);
>> + }
>> +
>> + phy_set_drvdata(iphy->phy, iphy);
>> + }
>> +
>> + dev_set_drvdata(dev, cbphy);
>> + phy_provider = devm_of_phy_provider_register(dev, intel_cbphy_xlate);
>> + if (IS_ERR(phy_provider))
>> + dev_err(dev, "Register PHY provider failed!\n");
>> +
>> + return PTR_ERR_OR_ZERO(phy_provider);
>> +}
>> +
>> +static int intel_cbphy_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct intel_combo_phy *cbphy;
>> + int ret;
>> +
>> + cbphy = devm_kzalloc(dev, sizeof(*cbphy), GFP_KERNEL);
>> + if (!cbphy)
>> + return -ENOMEM;
>> +
>> + cbphy->dev = dev;
>> + cbphy->init_cnt = 0;
>> + mutex_init(&cbphy->lock);
>> + ret = intel_cbphy_fwnode_parse(cbphy);
>> + if (ret)
>> + return ret;
>> +
>> + platform_set_drvdata(pdev, cbphy);
>> +
>> + return intel_cbphy_create(cbphy);
>> +}
>> +
>> +static int intel_cbphy_remove(struct platform_device *pdev)
>> +{
>> + struct intel_combo_phy *cbphy = platform_get_drvdata(pdev);
>> +
>> + intel_cbphy_rst_assert(cbphy);
>> + clk_disable_unprepare(cbphy->core_clk);
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id of_intel_cbphy_match[] = {
>> + { .compatible = "intel,combo-phy" },
>> + { .compatible = "intel,combophy-lgm" },
>> + {}
>> +};
>> +
>> +static struct platform_driver intel_cbphy_driver = {
>> + .probe = intel_cbphy_probe,
>> + .remove = intel_cbphy_remove,
>> + .driver = {
>> + .name = "intel-combo-phy",
>> + .of_match_table = of_intel_cbphy_match,
>> + }
>> +};
>> +
>> +module_platform_driver(intel_cbphy_driver);
>> +
>> +MODULE_DESCRIPTION("Intel Combo-phy driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.11.0
^ permalink raw reply
* Re: [PATCH v6 1/2] dt-bindings: mtd: Add Nand Flash Controller support for Intel LGM SoC
From: Ramuthevar, Vadivel MuruganX @ 2020-05-15 8:06 UTC (permalink / raw)
To: Rob Herring
Cc: masonccyang, linux-mtd, richard, boris.brezillon, brendanhiggins,
tglx, miquel.raynal, vigneshr, hauke.mehrtens, devicetree,
anders.roxell, arnd, robh+dt, andriy.shevchenko, linux-mips,
cheol.yong.kim, linux-kernel, qi-ming.wu
In-Reply-To: <dc51e6af-bda8-d8b9-1782-f5c4d5d3fed7@linux.intel.com>
Hi Rob,
On 15/5/2020 10:08 am, Ramuthevar, Vadivel MuruganX wrote:
> Hi Rob,
>
> On 14/5/2020 8:57 pm, Rob Herring wrote:
>> On Wed, 13 May 2020 18:46:14 +0800, Ramuthevar,Vadivel MuruganX wrote:
>>> From: Ramuthevar Vadivel Murugan
>>> <vadivel.muruganx.ramuthevar@linux.intel.com>
>>>
>>> Add YAML file for dt-bindings to support NAND Flash Controller
>>> on Intel's Lightning Mountain SoC.
>>>
>>> Signed-off-by: Ramuthevar Vadivel Murugan
>>> <vadivel.muruganx.ramuthevar@linux.intel.com>
>>> ---
>>> .../devicetree/bindings/mtd/intel,lgm-nand.yaml | 83
>>> ++++++++++++++++++++++
>>> 1 file changed, 83 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/mtd/intel,lgm-nand.yaml
>>>
>>
>>
>> My bot found errors running 'make dt_binding_check' on your patch:
>>
>> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mtd/intel,lgm-nand.example.dt.yaml:
>> nand-controller@e0f00000: 'dmas' is a dependency of 'dma-names'
>>
>> See https://patchwork.ozlabs.org/patch/1289160
>>
>> If you already ran 'make dt_binding_check' and didn't see the above
>> error(s), then make sure dt-schema is up to date:
>>
>> pip3 install
>> git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
>>
>> Please check and re-submit.Sure, will check and re-submit, Thanks!
Regards
Vadivel
^ permalink raw reply
* [PATCH 4/4] phy: omap-usb2-phy: disable PHY charger detect
From: Roger Quadros @ 2020-05-15 8:05 UTC (permalink / raw)
To: kishon; +Cc: robh+dt, b-liu, devicetree, vigneshr, nsekhar, Roger Quadros
In-Reply-To: <20200515080518.26870-1-rogerq@ti.com>
AM654x PG1.0 has a silicon bug that D+ is pulled high after POR, which
could cause enumeration failure with some USB hubs. Disabling the
USB2_PHY Charger Detect function will put D+ into the normal state.
Using property "ti,dis-chg-det-quirk" in the DT usb2-phy node to
enable this workaround for AM654x PG1.0.
This addresses Silicon Errata:
i2075 - "USB2PHY: USB2PHY Charger Detect is Enabled by Default Without VBUS
Presence"
Signed-off-by: Bin Liu <b-liu@ti.com>
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/phy/ti/phy-omap-usb2.c | 35 +++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/phy/ti/phy-omap-usb2.c b/drivers/phy/ti/phy-omap-usb2.c
index cb2dd3230fa7..8ab8b94511d4 100644
--- a/drivers/phy/ti/phy-omap-usb2.c
+++ b/drivers/phy/ti/phy-omap-usb2.c
@@ -26,6 +26,10 @@
#define USB2PHY_ANA_CONFIG1 0x4c
#define USB2PHY_DISCON_BYP_LATCH BIT(31)
+#define USB2PHY_CHRG_DET 0x14
+#define USB2PHY_CHRG_DET_USE_CHG_DET_REG BIT(29)
+#define USB2PHY_CHRG_DET_DIS_CHG_DET BIT(28)
+
/* SoC Specific USB2_OTG register definitions */
#define AM654_USB2_OTG_PD BIT(8)
#define AM654_USB2_VBUS_DET_EN BIT(5)
@@ -43,6 +47,7 @@
#define OMAP_USB2_HAS_START_SRP BIT(0)
#define OMAP_USB2_HAS_SET_VBUS BIT(1)
#define OMAP_USB2_CALIBRATE_FALSE_DISCONNECT BIT(2)
+#define OMAP_USB2_DISABLE_CHRG_DET BIT(3)
struct omap_usb {
struct usb_phy phy;
@@ -236,6 +241,13 @@ static int omap_usb_init(struct phy *x)
omap_usb_writel(phy->phy_base, USB2PHY_ANA_CONFIG1, val);
}
+ if (phy->flags & OMAP_USB2_DISABLE_CHRG_DET) {
+ val = omap_usb_readl(phy->phy_base, USB2PHY_CHRG_DET);
+ val |= USB2PHY_CHRG_DET_USE_CHG_DET_REG |
+ USB2PHY_CHRG_DET_DIS_CHG_DET;
+ omap_usb_writel(phy->phy_base, USB2PHY_CHRG_DET, val);
+ }
+
return 0;
}
@@ -366,14 +378,12 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy->mask = phy_data->mask;
phy->power_on = phy_data->power_on;
phy->power_off = phy_data->power_off;
+ phy->flags = phy_data->flags;
- if (phy_data->flags & OMAP_USB2_CALIBRATE_FALSE_DISCONNECT) {
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- phy->phy_base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(phy->phy_base))
- return PTR_ERR(phy->phy_base);
- phy->flags |= OMAP_USB2_CALIBRATE_FALSE_DISCONNECT;
- }
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ phy->phy_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(phy->phy_base))
+ return PTR_ERR(phy->phy_base);
phy->syscon_phy_power = syscon_regmap_lookup_by_phandle(node,
"syscon-phy-power");
@@ -405,6 +415,17 @@ static int omap_usb2_probe(struct platform_device *pdev)
}
}
+ /*
+ * Errata i2075: USB2PHY: USB2PHY Charger Detect is Enabled by
+ * Default Without VBUS Presence.
+ *
+ * AM654x SR1.0 has a silicon bug due to which D+ is pulled high after
+ * POR, which could cause enumeration failure with some USB hubs.
+ * Disabling the USB2_PHY Charger Detect function will put D+
+ * into the normal state.
+ */
+ if (of_property_read_bool(node, "ti,dis-chg-det-quirk"))
+ phy->flags |= OMAP_USB2_DISABLE_CHRG_DET;
phy->wkupclk = devm_clk_get(phy->dev, "wkupclk");
if (IS_ERR(phy->wkupclk)) {
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related
* [PATCH 3/4] dt-binding: phy: ti,omap-usb2: Add quirk to disable charger detection
From: Roger Quadros @ 2020-05-15 8:05 UTC (permalink / raw)
To: kishon; +Cc: robh+dt, b-liu, devicetree, vigneshr, nsekhar, Roger Quadros
In-Reply-To: <20200515080518.26870-1-rogerq@ti.com>
Add "ti,dis-chg-det-quirk" property to disable the USB2_PHY Charger Detect
logic.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml b/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
index ecfb28f714ea..ec3e85a17bfa 100644
--- a/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
+++ b/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
@@ -54,6 +54,11 @@ properties:
(deprecated) phandle of the control module used by PHY driver
to power on the PHY. Use syscon-phy-power instead.
+ ti,dis-chg-det-quirk:
+ description:
+ if present, driver will disable charger detection logic.
+ type: boolean
+
required:
- compatible
- reg
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related
* [PATCH 2/4] dt-binding: phy: convert ti,omap-usb2 to YAML
From: Roger Quadros @ 2020-05-15 8:05 UTC (permalink / raw)
To: kishon; +Cc: robh+dt, b-liu, devicetree, vigneshr, nsekhar, Roger Quadros
In-Reply-To: <20200515080518.26870-1-rogerq@ti.com>
Move ti,omap-usb2 to its own YAML schema.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
.../devicetree/bindings/phy/ti,omap-usb2.yaml | 73 +++++++++++++++++++
.../devicetree/bindings/phy/ti-phy.txt | 37 ----------
2 files changed, 73 insertions(+), 37 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
diff --git a/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml b/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
new file mode 100644
index 000000000000..ecfb28f714ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/ti,omap-usb2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: OMAP USB2 PHY
+
+maintainers:
+ - Kishon Vijay Abraham I <kishon@ti.com>
+ - Roger Quadros <rogerq@ti.com>
+
+properties:
+ compatible:
+ anyOf:
+ - items:
+ - enum:
+ - "ti,dra7x-usb2"
+ - "ti,dra7x-usb2-phy2"
+ - "ti,am654-usb2"
+ - enum:
+ - "ti,omap-usb2"
+
+ reg:
+ maxItems: 1
+ description: address and length of the register set for the device.
+
+ '#phy-cells':
+ description:
+ Number of cells in a PHY specifier. The meaning of all those
+ cells is defined by the binding for the phy node. The PHY
+ provider can use the values in cells to find the appropriate PHY.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ clocks:
+ minItems: 1
+ items:
+ - description: wakeup clock
+ - description: reference clock
+
+ clock-names:
+ minItems: 1
+ items:
+ - const: "wkupclk"
+ - const: "refclk"
+
+ syscon-phy-power:
+ description:
+ phandle/offset pair. Phandle to the system control module
+ register offset to power on/off the PHY.
+
+ ctrl-module:
+ description:
+ (deprecated) phandle of the control module used by PHY driver
+ to power on the PHY. Use syscon-phy-power instead.
+
+required:
+ - compatible
+ - reg
+ - '#phy-cells'
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ usb0_phy: phy@4100000 {
+ compatible = "ti,am654-usb2", "ti,omap-usb2";
+ reg = <0x0 0x4100000 0x0 0x54>;
+ syscon-phy-power = <&scm_conf 0x4000>;
+ clocks = <&k3_clks 151 0>, <&k3_clks 151 1>;
+ clock-names = "wkupclk", "refclk";
+ #phy-cells = <0>;
+ };
diff --git a/Documentation/devicetree/bindings/phy/ti-phy.txt b/Documentation/devicetree/bindings/phy/ti-phy.txt
index 8f93c3b694a7..60c9d0ac75e6 100644
--- a/Documentation/devicetree/bindings/phy/ti-phy.txt
+++ b/Documentation/devicetree/bindings/phy/ti-phy.txt
@@ -27,43 +27,6 @@ omap_control_usb: omap-control-usb@4a002300 {
reg-names = "otghs_control";
};
-OMAP USB2 PHY
-
-Required properties:
- - compatible: Should be "ti,omap-usb2"
- Should be "ti,dra7x-usb2" for the 1st instance of USB2 PHY on
- DRA7x
- Should be "ti,dra7x-usb2-phy2" for the 2nd instance of USB2 PHY
- in DRA7x
- Should be "ti,am654-usb2" for the USB2 PHYs on AM654.
- - reg : Address and length of the register set for the device.
- - #phy-cells: determine the number of cells that should be given in the
- phandle while referencing this phy.
- - clocks: a list of phandles and clock-specifier pairs, one for each entry in
- clock-names.
- - clock-names: should include:
- * "wkupclk" - wakeup clock.
- * "refclk" - reference clock (optional).
-
-Deprecated properties:
- - ctrl-module : phandle of the control module used by PHY driver to power on
- the PHY.
-
-Recommended properies:
-- syscon-phy-power : phandle/offset pair. Phandle to the system control
- module and the register offset to power on/off the PHY.
-
-This is usually a subnode of ocp2scp to which it is connected.
-
-usb2phy@4a0ad080 {
- compatible = "ti,omap-usb2";
- reg = <0x4a0ad080 0x58>;
- ctrl-module = <&omap_control_usb>;
- #phy-cells = <0>;
- clocks = <&usb_phy_cm_clk32k>, <&usb_otg_ss_refclk960m>;
- clock-names = "wkupclk", "refclk";
-};
-
TI PIPE3 PHY
Required properties:
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related
* [PATCH 1/4] phy: omap-usb2: Clean up exported header
From: Roger Quadros @ 2020-05-15 8:05 UTC (permalink / raw)
To: kishon; +Cc: robh+dt, b-liu, devicetree, vigneshr, nsekhar, Roger Quadros
In-Reply-To: <20200515080518.26870-1-rogerq@ti.com>
Move private definitions from header to phy-omap-usb2.c file.
Get rid of unused data structures usb_dpll_params and omap_usb_phy_type.
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
drivers/phy/ti/phy-omap-usb2.c | 60 +++++++++++++++++++++++++++--
include/linux/phy/omap_usb.h | 69 +---------------------------------
2 files changed, 58 insertions(+), 71 deletions(-)
diff --git a/drivers/phy/ti/phy-omap-usb2.c b/drivers/phy/ti/phy-omap-usb2.c
index 3d74629d7423..cb2dd3230fa7 100644
--- a/drivers/phy/ti/phy-omap-usb2.c
+++ b/drivers/phy/ti/phy-omap-usb2.c
@@ -1,8 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
- * omap-usb2.c - USB PHY, talking to musb controller in OMAP.
+ * omap-usb2.c - USB PHY, talking to USB controller on TI SoCs.
*
- * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2012-2020 Texas Instruments Incorporated - http://www.ti.com
* Author: Kishon Vijay Abraham I <kishon@ti.com>
*/
@@ -23,13 +23,65 @@
#include <linux/regmap.h>
#include <linux/of_platform.h>
-#define USB2PHY_DISCON_BYP_LATCH (1 << 31)
-#define USB2PHY_ANA_CONFIG1 0x4c
+#define USB2PHY_ANA_CONFIG1 0x4c
+#define USB2PHY_DISCON_BYP_LATCH BIT(31)
+/* SoC Specific USB2_OTG register definitions */
#define AM654_USB2_OTG_PD BIT(8)
#define AM654_USB2_VBUS_DET_EN BIT(5)
#define AM654_USB2_VBUSVALID_DET_EN BIT(4)
+#define OMAP_DEV_PHY_PD BIT(0)
+#define OMAP_USB2_PHY_PD BIT(28)
+
+#define AM437X_USB2_PHY_PD BIT(0)
+#define AM437X_USB2_OTG_PD BIT(1)
+#define AM437X_USB2_OTGVDET_EN BIT(19)
+#define AM437X_USB2_OTGSESSEND_EN BIT(20)
+
+/* Driver Flags */
+#define OMAP_USB2_HAS_START_SRP BIT(0)
+#define OMAP_USB2_HAS_SET_VBUS BIT(1)
+#define OMAP_USB2_CALIBRATE_FALSE_DISCONNECT BIT(2)
+
+struct omap_usb {
+ struct usb_phy phy;
+ struct phy_companion *comparator;
+ void __iomem *pll_ctrl_base;
+ void __iomem *phy_base;
+ struct device *dev;
+ struct device *control_dev;
+ struct clk *wkupclk;
+ struct clk *optclk;
+ u8 flags;
+ struct regmap *syscon_phy_power; /* ctrl. reg. acces */
+ unsigned int power_reg; /* power reg. index within syscon */
+ u32 mask;
+ u32 power_on;
+ u32 power_off;
+};
+
+#define phy_to_omapusb(x) container_of((x), struct omap_usb, phy)
+
+struct usb_phy_data {
+ const char *label;
+ u8 flags;
+ u32 mask;
+ u32 power_on;
+ u32 power_off;
+};
+
+static inline u32 omap_usb_readl(void __iomem *addr, unsigned int offset)
+{
+ return __raw_readl(addr + offset);
+}
+
+static inline void omap_usb_writel(void __iomem *addr, unsigned int offset,
+ u32 data)
+{
+ __raw_writel(data, addr + offset);
+}
+
/**
* omap_usb2_set_comparator - links the comparator present in the sytem with
* this phy
diff --git a/include/linux/phy/omap_usb.h b/include/linux/phy/omap_usb.h
index 5973a6313529..e23b52df93ec 100644
--- a/include/linux/phy/omap_usb.h
+++ b/include/linux/phy/omap_usb.h
@@ -2,68 +2,14 @@
/*
* omap_usb.h -- omap usb2 phy header file
*
- * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com
+ * Copyright (C) 2012-2020 Texas Instruments Incorporated - http://www.ti.com
* Author: Kishon Vijay Abraham I <kishon@ti.com>
*/
#ifndef __DRIVERS_OMAP_USB2_H
#define __DRIVERS_OMAP_USB2_H
-#include <linux/io.h>
-#include <linux/usb/otg.h>
-
-struct usb_dpll_params {
- u16 m;
- u8 n;
- u8 freq:3;
- u8 sd;
- u32 mf;
-};
-
-enum omap_usb_phy_type {
- TYPE_USB2, /* USB2_PHY, power down in CONTROL_DEV_CONF */
- TYPE_DRA7USB2, /* USB2 PHY, power and power_aux e.g. DRA7 */
- TYPE_AM437USB2, /* USB2 PHY, power e.g. AM437x */
-};
-
-struct omap_usb {
- struct usb_phy phy;
- struct phy_companion *comparator;
- void __iomem *pll_ctrl_base;
- void __iomem *phy_base;
- struct device *dev;
- struct device *control_dev;
- struct clk *wkupclk;
- struct clk *optclk;
- u8 flags;
- enum omap_usb_phy_type type;
- struct regmap *syscon_phy_power; /* ctrl. reg. acces */
- unsigned int power_reg; /* power reg. index within syscon */
- u32 mask;
- u32 power_on;
- u32 power_off;
-};
-
-struct usb_phy_data {
- const char *label;
- u8 flags;
- u32 mask;
- u32 power_on;
- u32 power_off;
-};
-
-/* Driver Flags */
-#define OMAP_USB2_HAS_START_SRP (1 << 0)
-#define OMAP_USB2_HAS_SET_VBUS (1 << 1)
-#define OMAP_USB2_CALIBRATE_FALSE_DISCONNECT (1 << 2)
-
-#define OMAP_DEV_PHY_PD BIT(0)
-#define OMAP_USB2_PHY_PD BIT(28)
-
-#define AM437X_USB2_PHY_PD BIT(0)
-#define AM437X_USB2_OTG_PD BIT(1)
-#define AM437X_USB2_OTGVDET_EN BIT(19)
-#define AM437X_USB2_OTGSESSEND_EN BIT(20)
+#include <linux/usb/phy_companion.h>
#define phy_to_omapusb(x) container_of((x), struct omap_usb, phy)
@@ -76,15 +22,4 @@ static inline int omap_usb2_set_comparator(struct phy_companion *comparator)
}
#endif
-static inline u32 omap_usb_readl(void __iomem *addr, unsigned offset)
-{
- return __raw_readl(addr + offset);
-}
-
-static inline void omap_usb_writel(void __iomem *addr, unsigned offset,
- u32 data)
-{
- __raw_writel(data, addr + offset);
-}
-
#endif /* __DRIVERS_OMAP_USB_H */
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply related
* [PATCH 0/4] phy: omap-usb2: add quirk to disable charger detection
From: Roger Quadros @ 2020-05-15 8:05 UTC (permalink / raw)
To: kishon; +Cc: robh+dt, b-liu, devicetree, vigneshr, nsekhar, Roger Quadros
Hi,
This series does 3 things
- header cleanup
- convert DT binding to YAML
- add DT property to disable charger detection (Errata i2075 for AM65 SR1.0)
cheers,
-roger
Roger Quadros (4):
phy: omap-usb2: Clean up exported header
dt-binding: phy: convert ti,omap-usb2 to YAML
dt-binding: phy: ti,omap-usb2: Add quirk to disable charger detection
phy: omap-usb2-phy: disable PHY charger detect
.../devicetree/bindings/phy/ti,omap-usb2.yaml | 78 +++++++++++++++
.../devicetree/bindings/phy/ti-phy.txt | 37 --------
drivers/phy/ti/phy-omap-usb2.c | 95 ++++++++++++++++---
include/linux/phy/omap_usb.h | 69 +-------------
4 files changed, 164 insertions(+), 115 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/ti,omap-usb2.yaml
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
From: H. Nikolaus Schaller @ 2020-05-15 7:59 UTC (permalink / raw)
To: Rob Herring
Cc: David Airlie, Daniel Vetter, Mark Rutland, Benoît Cousson,
Tony Lindgren, Paul Cercueil, Ralf Baechle, Paul Burton,
James Hogan, Kukjin Kim, Krzysztof Kozlowski, Maxime Ripard,
Chen-Yu Tsai, Thomas Bogendoerfer, Jonathan Bakker,
Philipp Rossak, dri-devel, devicetree, linux-kernel, linux-omap,
openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
linux-arm-kernel, linux-samsung-soc
In-Reply-To: <20200505155311.GA18025@bogus>
> Am 05.05.2020 um 17:53 schrieb Rob Herring <robh@kernel.org>:
>
> On Fri, Apr 24, 2020 at 10:34:04PM +0200, H. Nikolaus Schaller wrote:
>> The Imagination PVR/SGX GPU is part of several SoC from
>> multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo,
>> Allwinner A83 and others.
>>
>> With this binding, we describe how the SGX processor is
>> interfaced to the SoC (registers and interrupt).
>>
>> The interface also consists of clocks, reset, power but
>> information from data sheets is vague and some SoC integrators
>> (TI) deciced to use a PRCM wrapper (ti,sysc) which does
>> all clock, reset and power-management through registers
>> outside of the sgx register block.
>>
>> Therefore all these properties are optional.
>>
>> Tested by make dt_binding_check
>>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> .../devicetree/bindings/gpu/img,pvrsgx.yaml | 150 ++++++++++++++++++
>> 1 file changed, 150 insertions(+)
>> + oneOf:
>> + - description: SGX530-121 based SoC
>> + items:
>> + - enum:
>> + - ti,omap3-sgx530-121 # BeagleBoard A/B/C, OpenPandora 600MHz and similar
>
> Should be indented 2 more here and elsewhere where you have a list
> under a list.
added for patch v8 series.
BR and thanks,
Nikolaus
^ permalink raw reply
* Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
From: H. Nikolaus Schaller @ 2020-05-15 7:58 UTC (permalink / raw)
To: Tony Lindgren
Cc: Paul Cercueil, David Airlie, Daniel Vetter, Rob Herring,
Mark Rutland, Benoît Cousson, Ralf Baechle, Paul Burton,
James Hogan, Kukjin Kim, Krzysztof Kozlowski, Maxime Ripard,
Chen-Yu Tsai, Thomas Bogendoerfer, Jonathan Bakker,
Philipp Rossak, dri-devel, devicetree, linux-kernel, linux-omap,
openpvrsgx-devgroup, letux-kernel, kernel, linux-mips,
linux-arm-kernel, linux-samsung-soc
In-Reply-To: <20200503150143.GG37466@atomide.com>
Hi Tony,
> Am 03.05.2020 um 17:01 schrieb Tony Lindgren <tony@atomide.com>:
>
> * Paul Cercueil <paul@crapouillou.net> [200503 14:19]:
>> You have a new SoC with a SGX, and you only need to enable one clock to get
>> it to work. So you create a devicetree node which receives only one clock.
>>
>> Turns out, that the bootloader was enabling the other 3 clocks, and since
>> the last release, it doesn't anymore. You're left with having to support a
>> broken devicetree.
>>
>> That's the kind of problem that can be easily avoided by enforcing the
>> number of clocks that have to be provided.
>
> The number of clocks depends on how it's wired for the SoC.
>
> On omaps, there's are no controls for additinoal SGX clocks. Sure some
> of the clocks may be routed to multple places internally by the wrapper
> module. But we have no control over that.
>
> If we wanted to specify just the "fck" clock on omaps, then we can
> do it with something like this:
>
> allOf:
> - if:
> properites:
> compatible:
> enum:
> - "ti,omap4-sgx544-112"
> - "ti,omap5-sgx544-116"
> - "ti,dra7-sgx544-116"
> then:
> properties:
> clocks:
> minItems: 1
> maxItems: 1
>
> clock-names:
> const: fck
>
> required:
> - clocks
> - clock-names
will add to v8 of this series as a separate patch on top of the
general one. This should make it easier to have a focussed discussion
and revert/bisect if something goes wrong.
BR and thanks,
Nikolaus
^ permalink raw reply
* Re: [PATCH v2 0/6] phy: meson8b-usb2: small fixes and improvements
From: Vinod Koul @ 2020-05-15 7:49 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: kishon, robh+dt, devicetree, linux-amlogic, narmstrong,
linux-arm-kernel, linux-kernel, hexdump0815
In-Reply-To: <20200512222424.549351-1-martin.blumenstingl@googlemail.com>
On 13-05-20, 00:24, Martin Blumenstingl wrote:
> This is a batch of fixes and improvements for the phy-meson8b-usb2
> driver:
> - convert the existing dt-bindings to json-schema and add a fallback
> compatible string which is already in existing .dtsi files
> - differentiate between Meson8 and Meson8m2 using a new compatible
> string for the latter
> - simplify the code by using a MMIO regmap
> - set / unset the IDDQ and ACA enable bits depending on the operating
> mode (and for the latter also the compatible string)
>
> I suggest that all of these are applied to -next because we will need a
> separate .dts patch (which I already have prepared) to use the new
> Meson8m2 compatible string in meson8m2.dtsi. Otherwise we will be
> changing behavior in patch #4, because meson8m2.dtsi currently inherits
> the Meson8 compatible string.
> The number of actual Meson8 users is very small (I only know one case),
> so keeping Meson8m2 working is more important to me (because I know
> several users with boards that have a Meson8m2 SoC).
Applied all, thanks
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
From: Serge Semin @ 2020-05-15 7:48 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Greg Kroah-Hartman, Arnd Bergmann, Rob Herring, linux-pm,
devicetree, Vincenzo Frascino, Thomas Gleixner, linux-mips,
linux-kernel
In-Reply-To: <20200511133121.cz5axbwynhmqkx7x@mobilestation>
Thomas,
Could you take a look at my comment below so I could proceed with the
patchset v3 development?
-Sergey
On Mon, May 11, 2020 at 04:31:21PM +0300, Serge Semin wrote:
> On Fri, May 08, 2020 at 05:41:50PM +0200, Thomas Bogendoerfer wrote:
> > On Wed, May 06, 2020 at 08:42:36PM +0300, Sergey.Semin@baikalelectronics.ru wrote:
> > > From: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > >
> > > Commit 07d69579e7fe ("MIPS: Don't register r4k sched clock when
> > > CPUFREQ enabled") disabled the r4k-clock usage for scheduler ticks
> > > counting due to the scheduler being non-tolerant for unstable
> > > clocks sources. For the same reason the clock should be used
> > > in the system clocksource framework only as a last resort if CPU
> > > frequency may change.
> > >
> > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> > > Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> > > Cc: Paul Burton <paulburton@kernel.org>
> > > Cc: Ralf Baechle <ralf@linux-mips.org>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: linux-pm@vger.kernel.org
> > > Cc: devicetree@vger.kernel.org
> > > ---
> > > arch/mips/kernel/csrc-r4k.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
> > > index 437dda64fd7a..d81fb374f477 100644
> > > --- a/arch/mips/kernel/csrc-r4k.c
> > > +++ b/arch/mips/kernel/csrc-r4k.c
> > > @@ -71,7 +71,11 @@ int __init init_r4k_clocksource(void)
> > > return -ENXIO;
> > >
> > > /* Calculate a somewhat reasonable rating value */
> > > +#ifndef CONFIG_CPU_FREQ
> > > clocksource_mips.rating = 200 + mips_hpt_frequency / 10000000;
> > > +#else
> > > + clocksource_mips.rating = 99;
> > > +#endif
> >
> > I dislike this patch. Assuming you have an other clocksource, why not
> > simply disable csrc-r4k, if CPU_FREQ is enabled ?
>
> Me neither and the best way would be to update the clocksource frequency
> dynamically the same way it's done for cevt-r4k and MIPS GIC timers. Alas the
> clocksource doesn't support it. Due to this together with CPU-freq facility
> enabled we have to use a very slow DW APB Timer instead of the fast embedded
> into the CPU core r4k and MIPS GIC timers. Just note the difference: it takes
> 220 ns to read the counter from DW APB Timer in comparison to a few nanoseconds
> reading from MIPS GIC and R4K. So IMO disabling the timer as you suggest isn't
> the best option. By making the CPUFREQ and CSRC_R4K mutual exclusive we'd
> assume a use-case that the system will always use the CPU-freq facility changing
> the CPU reference frequency. This is obviously not true. Noone prevents the
> system administrator to leave the default setting of the CPU-freq with fixed
> frequency and select a faster, more accurate timer like in our case.
>
> My idea was not to try to predict how the system would be used, but to let the
> system administration to choose which timer is applicable in particular usecase
> enabling a safest one by default. So if CPUFREQ is available, then we fallback
> to the external timer as safest one. If the system user wouldn't need to have
> the CPUFREQ facility utilized, then the system administrator would want to
> leave the default CPU-freq governor with pre-defined CPU frequency and
> select either R4K (MIPS) or MIPS GIC timer just by writing its name into
> /sys/bus/clocksource/devices/clocksource0/current_clocksource .
>
> I should note, that currently CPU_FREQ won't be available if there is no
> MIPS_EXTERNAL_TIMER available for the platform. It's prohibited by means of the
> conditional kbuild config inclusion declared in the arch/mips/Kconfig:
> + if CPU_SUPPORTS_CPUFREQ && MIPS_EXTERNAL_TIMER
> + source "drivers/cpufreq/Kconfig"
> + endif
> So if there is no external timer working independently from the CPU core clock
> source, the CPUFREQ won't be available to select for the kernel. Though currently
> this limitation is supposed to be applicable for the R4K/MIPS GIC clocksource
> timers only since clockevents must work fine in unstable reference clock conditions.
>
> So what can we do to improve the patch? First one is a solution I suggested in
> this patch but it could be a bit altered by using IS_ENABLED() macro to:
> + clocksource_mips.rating = !IS_ENABLED(CONFIG_CPU_FREQ) ?
> + 200 + mips_hpt_frequency / 10000000 : 99;
>
> Another idea I discovered when have been searching through the x86 arch code.
> x86's got the same problem with TSC timer, but it doesn't disable it if
> CPU-frequency is switched on. Instead it just marks it as unstable by calling
> the clocksource_mark_unstable() method if CPU frequency changes. I suggest to
> implement the same approach in our case of MIPS GIC (another patchset
> I've sent, see "clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC
> support" in your email client) and R4K timers. We'll subscribe to the CPU
> frequency change and if it changes we'll call clocksource_mark_unstable() with
> MIPS GIC and R4K clocksource handlers passed. This shall reduce their rating and
> cause selecting a clocksource with better one. BTW I suppose it won't be
> necessary to initially lower the rating of the MIPS GIC and R4K clocksource
> timers if this is implemented.
>
> So, what do you think?
>
> -Sergey
>
> >
> > Thomas.
> >
> > --
> > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> > good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v8 3/3] phy: intel: Add driver support for ComboPhy
From: Vinod Koul @ 2020-05-15 7:39 UTC (permalink / raw)
To: Dilip Kota
Cc: linux-kernel, kishon, devicetree, robh, andriy.shevchenko,
cheol.yong.kim, chuanhua.lei, qi-ming.wu, yixin.zhu
In-Reply-To: <af8a7d7025990d22f6062953247cf80e64c6fd2f.1589516297.git.eswara.kota@linux.intel.com>
On 15-05-20, 12:30, Dilip Kota wrote:
> ComboPhy subsystem provides PHYs for various
> controllers like PCIe, SATA and EMAC.
>
> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
> ---
> Changes on v8:
> As per PHY Maintainer's request add description for doing register access
> through regmap in comments.
I dont see any comments below, and my script tells me this is *same* as
v7, did you send the wrong version?
>
> Changes on v7:
> Use device_node_to_regmap instead of fwnode_to_regmap
>
> Changes on v6:
> No changes
>
> Changes on v5:
> Add changes as per inputs from Andy and Rob:
> DT node uses phy-mode values as defined in "include/dt-bindings/phy/phy.h",
> add changes to handle it.
> ComboPhy no longer has children nodes, and children node properties(reset)
> moved to parent node, so do the code changes accordingly.
> Add _xlate() function to pass the appropriate phy handle.
> Fix couple of nitpicks.
>
> Changes on v4:
> Address review comments
> Remove dependency on OF config
> Update copyright to 2019-2020
> Define register macro PAD_DIS_CFG instead of const variable inside function.
> Improve the error prints, and error returns.
> Call put_device(dev), for get_dev_from_fwnode()
> Move platform_set_drvdata() at the end of the probe().
> Correct alignment in phy_ops intel_cbphy_ops.
> Correct commented lines with proper vocabulary and punctuation.
> Add/remove commas for the required constant arrays and enums.
> Remove in driver:
> linux/kernel.h, not required
> macros: PCIE_PHY_MPLLA_CTRL, PCIE_PHY_MPLLB_CTRL
> temp variable u32 prop;
> Change function names:
> intel_cbphy_iphy_dt_parse() -> intel_cbphy_iphy_fwnode_parse()
> intel_cbphy_dt_sanity_check() -> intel_cbphy_sanity_check()
> intel_cbphy_dt_parse() -> intel_cbphy_fwnode_parse()
>
> Changes on v3:
> Remove intel_iphy_names
> Remove struct phy in struct intel_cbphy_iphy
> Imporve if conditions logic
> Use fwnode_to_regmap()
> Call devm_of_platform_populate() to populate child nodes
> Fix reset sequence during phy_init
> Add SoC specific compatible "intel,combophy-lgm"
> Add description for enums
> Remove default case in switch {} intel_cbphy_set_mode() as it
> never happens.
> Use mutex_lock to synchronise combophy initialization across
> two phys.
> Change init_cnt to u32 datatype as it is within mutex lock.
> Correct error handling of
> fwnode_property_read_u32_array(fwnode, "intel,phy-mode", ...)
>
> drivers/phy/intel/Kconfig | 14 +
> drivers/phy/intel/Makefile | 1 +
> drivers/phy/intel/phy-intel-combo.c | 627 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 642 insertions(+)
> create mode 100644 drivers/phy/intel/phy-intel-combo.c
>
> diff --git a/drivers/phy/intel/Kconfig b/drivers/phy/intel/Kconfig
> index 4ea6a8897cd7..3b40eb7b4fb4 100644
> --- a/drivers/phy/intel/Kconfig
> +++ b/drivers/phy/intel/Kconfig
> @@ -2,6 +2,20 @@
> #
> # Phy drivers for Intel Lightning Mountain(LGM) platform
> #
> +config PHY_INTEL_COMBO
> + bool "Intel ComboPHY driver"
> + depends on X86 || COMPILE_TEST
> + depends on OF && HAS_IOMEM
> + select MFD_SYSCON
> + select GENERIC_PHY
> + select REGMAP
> + help
> + Enable this to support Intel ComboPhy.
> +
> + This driver configures ComboPhy subsystem on Intel gateway
> + chipsets which provides PHYs for various controllers, EMAC,
> + SATA and PCIe.
> +
> config PHY_INTEL_EMMC
> tristate "Intel EMMC PHY driver"
> select GENERIC_PHY
> diff --git a/drivers/phy/intel/Makefile b/drivers/phy/intel/Makefile
> index 6b876a75599d..233d530dadde 100644
> --- a/drivers/phy/intel/Makefile
> +++ b/drivers/phy/intel/Makefile
> @@ -1,2 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_PHY_INTEL_COMBO) += phy-intel-combo.o
> obj-$(CONFIG_PHY_INTEL_EMMC) += phy-intel-emmc.o
> diff --git a/drivers/phy/intel/phy-intel-combo.c b/drivers/phy/intel/phy-intel-combo.c
> new file mode 100644
> index 000000000000..04ad595e21e4
> --- /dev/null
> +++ b/drivers/phy/intel/phy-intel-combo.c
> @@ -0,0 +1,627 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Intel Combo-PHY driver
> + *
> + * Copyright (C) 2019-2020 Intel Corporation.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/iopoll.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include <dt-bindings/phy/phy.h>
> +
> +#define PCIE_PHY_GEN_CTRL 0x00
> +#define PCIE_PHY_CLK_PAD BIT(17)
> +
> +#define PAD_DIS_CFG 0x174
> +
> +#define PCS_XF_ATE_OVRD_IN_2 0x3008
> +#define ADAPT_REQ_MSK GENMASK(5, 4)
> +
> +#define PCS_XF_RX_ADAPT_ACK 0x3010
> +#define RX_ADAPT_ACK_BIT BIT(0)
> +
> +#define CR_ADDR(addr, lane) (((addr) + (lane) * 0x100) << 2)
> +#define REG_COMBO_MODE(x) ((x) * 0x200)
> +#define REG_CLK_DISABLE(x) ((x) * 0x200 + 0x124)
> +
> +#define COMBO_PHY_ID(x) ((x)->parent->id)
> +#define PHY_ID(x) ((x)->id)
> +
> +#define CLK_100MHZ 100000000
> +#define CLK_156_25MHZ 156250000
> +
> +static const unsigned long intel_iphy_clk_rates[] = {
> + CLK_100MHZ, CLK_156_25MHZ, CLK_100MHZ,
> +};
> +
> +enum {
> + PHY_0,
> + PHY_1,
> + PHY_MAX_NUM
> +};
> +
> +/*
> + * Clock Register bit fields to enable clocks
> + * for ComboPhy according to the mode.
> + */
> +enum intel_phy_mode {
> + PHY_PCIE_MODE = 0,
> + PHY_XPCS_MODE,
> + PHY_SATA_MODE,
> +};
> +
> +/* ComboPhy mode Register values */
> +enum intel_combo_mode {
> + PCIE0_PCIE1_MODE = 0,
> + PCIE_DL_MODE,
> + RXAUI_MODE,
> + XPCS0_XPCS1_MODE,
> + SATA0_SATA1_MODE,
> +};
> +
> +enum aggregated_mode {
> + PHY_SL_MODE,
> + PHY_DL_MODE,
> +};
> +
> +struct intel_combo_phy;
> +
> +struct intel_cbphy_iphy {
> + struct phy *phy;
> + struct intel_combo_phy *parent;
> + struct reset_control *app_rst;
> + u32 id;
> +};
> +
> +struct intel_combo_phy {
> + struct device *dev;
> + struct clk *core_clk;
> + unsigned long clk_rate;
> + void __iomem *app_base;
> + void __iomem *cr_base;
> + struct regmap *syscfg;
> + struct regmap *hsiocfg;
> + u32 id;
> + u32 bid;
> + struct reset_control *phy_rst;
> + struct reset_control *core_rst;
> + struct intel_cbphy_iphy iphy[PHY_MAX_NUM];
> + enum intel_phy_mode phy_mode;
> + enum aggregated_mode aggr_mode;
> + u32 init_cnt;
> + struct mutex lock;
> +};
> +
> +static int intel_cbphy_iphy_enable(struct intel_cbphy_iphy *iphy, bool set)
> +{
> + struct intel_combo_phy *cbphy = iphy->parent;
> + u32 mask = BIT(cbphy->phy_mode * 2 + iphy->id);
> + u32 val;
> +
> + /* Register: 0 is enable, 1 is disable */
> + val = set ? 0 : mask;
> +
> + return regmap_update_bits(cbphy->hsiocfg, REG_CLK_DISABLE(cbphy->bid),
> + mask, val);
> +}
> +
> +static int intel_cbphy_pcie_refclk_cfg(struct intel_cbphy_iphy *iphy, bool set)
> +{
> + struct intel_combo_phy *cbphy = iphy->parent;
> + u32 mask = BIT(cbphy->id * 2 + iphy->id);
> + u32 val;
> +
> + /* Register: 0 is enable, 1 is disable */
> + val = set ? 0 : mask;
> +
> + return regmap_update_bits(cbphy->syscfg, PAD_DIS_CFG, mask, val);
> +}
> +
> +static inline void combo_phy_w32_off_mask(void __iomem *base, unsigned int reg,
> + u32 mask, u32 val)
> +{
> + u32 reg_val;
> +
> + reg_val = readl(base + reg);
> + reg_val &= ~mask;
> + reg_val |= FIELD_PREP(mask, val);
> + writel(reg_val, base + reg);
> +}
> +
> +static int intel_cbphy_iphy_cfg(struct intel_cbphy_iphy *iphy,
> + int (*phy_cfg)(struct intel_cbphy_iphy *))
> +{
> + struct intel_combo_phy *cbphy = iphy->parent;
> + int ret;
> +
> + ret = phy_cfg(iphy);
> + if (ret)
> + return ret;
> +
> + if (cbphy->aggr_mode != PHY_DL_MODE)
> + return 0;
> +
> + return phy_cfg(&cbphy->iphy[PHY_1]);
> +}
> +
> +static int intel_cbphy_pcie_en_pad_refclk(struct intel_cbphy_iphy *iphy)
> +{
> + struct intel_combo_phy *cbphy = iphy->parent;
> + int ret;
> +
> + ret = intel_cbphy_pcie_refclk_cfg(iphy, true);
> + if (ret) {
> + dev_err(cbphy->dev, "Failed to enable PCIe pad refclk\n");
> + return ret;
> + }
> +
> + if (cbphy->init_cnt)
> + return 0;
> +
> + combo_phy_w32_off_mask(cbphy->app_base, PCIE_PHY_GEN_CTRL,
> + PCIE_PHY_CLK_PAD, 0);
> +
> + /* Delay for stable clock PLL */
> + usleep_range(50, 100);
> +
> + return 0;
> +}
> +
> +static int intel_cbphy_pcie_dis_pad_refclk(struct intel_cbphy_iphy *iphy)
> +{
> + struct intel_combo_phy *cbphy = iphy->parent;
> + int ret;
> +
> + ret = intel_cbphy_pcie_refclk_cfg(iphy, false);
> + if (ret) {
> + dev_err(cbphy->dev, "Failed to disable PCIe pad refclk\n");
> + return ret;
> + }
> +
> + if (cbphy->init_cnt)
> + return 0;
> +
> + combo_phy_w32_off_mask(cbphy->app_base, PCIE_PHY_GEN_CTRL,
> + PCIE_PHY_CLK_PAD, 1);
> +
> + return 0;
> +}
> +
> +static int intel_cbphy_set_mode(struct intel_combo_phy *cbphy)
> +{
> + enum aggregated_mode aggr = cbphy->aggr_mode;
> + struct device *dev = cbphy->dev;
> + enum intel_combo_mode cb_mode;
> + enum intel_phy_mode mode;
> + int ret;
> +
> + mode = cbphy->phy_mode;
> +
> + switch (mode) {
> + case PHY_PCIE_MODE:
> + cb_mode = (aggr == PHY_DL_MODE) ? PCIE_DL_MODE : PCIE0_PCIE1_MODE;
> + break;
> +
> + case PHY_XPCS_MODE:
> + cb_mode = (aggr == PHY_DL_MODE) ? RXAUI_MODE : XPCS0_XPCS1_MODE;
> + break;
> +
> + case PHY_SATA_MODE:
> + if (aggr == PHY_DL_MODE) {
> + dev_err(dev, "Mode:%u not support dual lane!\n", mode);
> + return -EINVAL;
> + }
> +
> + cb_mode = SATA0_SATA1_MODE;
> + break;
> + }
> +
> + ret = regmap_write(cbphy->hsiocfg, REG_COMBO_MODE(cbphy->bid), cb_mode);
> + if (ret)
> + dev_err(dev, "Failed to set ComboPhy mode: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static void intel_cbphy_rst_assert(struct intel_combo_phy *cbphy)
> +{
> + reset_control_assert(cbphy->core_rst);
> + reset_control_assert(cbphy->phy_rst);
> +}
> +
> +static void intel_cbphy_rst_deassert(struct intel_combo_phy *cbphy)
> +{
> + reset_control_deassert(cbphy->core_rst);
> + reset_control_deassert(cbphy->phy_rst);
> + /* Delay to ensure reset process is done */
> + usleep_range(10, 20);
> +}
> +
> +static int intel_cbphy_iphy_power_on(struct intel_cbphy_iphy *iphy)
> +{
> + struct intel_combo_phy *cbphy = iphy->parent;
> + int ret;
> +
> + if (!cbphy->init_cnt) {
> + ret = clk_prepare_enable(cbphy->core_clk);
> + if (ret) {
> + dev_err(cbphy->dev, "Clock enable failed!\n");
> + return ret;
> + }
> +
> + ret = clk_set_rate(cbphy->core_clk, cbphy->clk_rate);
> + if (ret) {
> + dev_err(cbphy->dev, "Clock freq set to %lu failed!\n",
> + cbphy->clk_rate);
> + goto clk_err;
> + }
> +
> + intel_cbphy_rst_assert(cbphy);
> + intel_cbphy_rst_deassert(cbphy);
> + ret = intel_cbphy_set_mode(cbphy);
> + if (ret)
> + goto clk_err;
> + }
> +
> + ret = intel_cbphy_iphy_enable(iphy, true);
> + if (ret) {
> + dev_err(cbphy->dev, "Failed enabling PHY core\n");
> + goto clk_err;
> + }
> +
> + ret = reset_control_deassert(iphy->app_rst);
> + if (ret) {
> + dev_err(cbphy->dev, "PHY(%u:%u) reset deassert failed!\n",
> + COMBO_PHY_ID(iphy), PHY_ID(iphy));
> + goto clk_err;
> + }
> +
> + /* Delay to ensure reset process is done */
> + udelay(1);
> +
> + return 0;
> +
> +clk_err:
> + clk_disable_unprepare(cbphy->core_clk);
> +
> + return ret;
> +}
> +
> +static int intel_cbphy_iphy_power_off(struct intel_cbphy_iphy *iphy)
> +{
> + struct intel_combo_phy *cbphy = iphy->parent;
> + int ret;
> +
> + ret = reset_control_assert(iphy->app_rst);
> + if (ret) {
> + dev_err(cbphy->dev, "PHY(%u:%u) reset assert failed!\n",
> + COMBO_PHY_ID(iphy), PHY_ID(iphy));
> + return ret;
> + }
> +
> + ret = intel_cbphy_iphy_enable(iphy, false);
> + if (ret) {
> + dev_err(cbphy->dev, "Failed disabling PHY core\n");
> + return ret;
> + }
> +
> + if (cbphy->init_cnt)
> + return 0;
> +
> + clk_disable_unprepare(cbphy->core_clk);
> + intel_cbphy_rst_assert(cbphy);
> +
> + return 0;
> +}
> +
> +static int intel_cbphy_init(struct phy *phy)
> +{
> + struct intel_cbphy_iphy *iphy = phy_get_drvdata(phy);
> + struct intel_combo_phy *cbphy = iphy->parent;
> + int ret;
> +
> + mutex_lock(&cbphy->lock);
> + ret = intel_cbphy_iphy_cfg(iphy, intel_cbphy_iphy_power_on);
> + if (ret)
> + goto err;
> +
> + if (cbphy->phy_mode == PHY_PCIE_MODE) {
> + ret = intel_cbphy_iphy_cfg(iphy, intel_cbphy_pcie_en_pad_refclk);
> + if (ret)
> + goto err;
> + }
> +
> + cbphy->init_cnt++;
> +
> +err:
> + mutex_unlock(&cbphy->lock);
> +
> + return ret;
> +}
> +
> +static int intel_cbphy_exit(struct phy *phy)
> +{
> + struct intel_cbphy_iphy *iphy = phy_get_drvdata(phy);
> + struct intel_combo_phy *cbphy = iphy->parent;
> + int ret;
> +
> + mutex_lock(&cbphy->lock);
> + cbphy->init_cnt--;
> + if (cbphy->phy_mode == PHY_PCIE_MODE) {
> + ret = intel_cbphy_iphy_cfg(iphy, intel_cbphy_pcie_dis_pad_refclk);
> + if (ret)
> + goto err;
> + }
> +
> + ret = intel_cbphy_iphy_cfg(iphy, intel_cbphy_iphy_power_off);
> +
> +err:
> + mutex_unlock(&cbphy->lock);
> +
> + return ret;
> +}
> +
> +static int intel_cbphy_calibrate(struct phy *phy)
> +{
> + struct intel_cbphy_iphy *iphy = phy_get_drvdata(phy);
> + struct intel_combo_phy *cbphy = iphy->parent;
> + void __iomem *cr_base = cbphy->cr_base;
> + int val, ret, id;
> +
> + if (cbphy->phy_mode != PHY_XPCS_MODE)
> + return 0;
> +
> + id = PHY_ID(iphy);
> +
> + /* trigger auto RX adaptation */
> + combo_phy_w32_off_mask(cr_base, CR_ADDR(PCS_XF_ATE_OVRD_IN_2, id),
> + ADAPT_REQ_MSK, 3);
> + /* Wait RX adaptation to finish */
> + ret = readl_poll_timeout(cr_base + CR_ADDR(PCS_XF_RX_ADAPT_ACK, id),
> + val, val & RX_ADAPT_ACK_BIT, 10, 5000);
> + if (ret)
> + dev_err(cbphy->dev, "RX Adaptation failed!\n");
> + else
> + dev_dbg(cbphy->dev, "RX Adaptation success!\n");
> +
> + /* Stop RX adaptation */
> + combo_phy_w32_off_mask(cr_base, CR_ADDR(PCS_XF_ATE_OVRD_IN_2, id),
> + ADAPT_REQ_MSK, 0);
> +
> + return ret;
> +}
> +
> +static int intel_cbphy_fwnode_parse(struct intel_combo_phy *cbphy)
> +{
> + struct device *dev = cbphy->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + struct fwnode_reference_args ref;
> + int ret;
> + u32 val;
> +
> + cbphy->core_clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(cbphy->core_clk)) {
> + ret = PTR_ERR(cbphy->core_clk);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Get clk failed:%d!\n", ret);
> + return ret;
> + }
> +
> + cbphy->core_rst = devm_reset_control_get_optional(dev, "core");
> + if (IS_ERR(cbphy->core_rst)) {
> + ret = PTR_ERR(cbphy->core_rst);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Get core reset control err: %d!\n", ret);
> + return ret;
> + }
> +
> + cbphy->phy_rst = devm_reset_control_get_optional(dev, "phy");
> + if (IS_ERR(cbphy->phy_rst)) {
> + ret = PTR_ERR(cbphy->phy_rst);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Get PHY reset control err: %d!\n", ret);
> + return ret;
> + }
> +
> + cbphy->iphy[0].app_rst = devm_reset_control_get_optional(dev, "iphy0");
> + if (IS_ERR(cbphy->iphy[0].app_rst)) {
> + ret = PTR_ERR(cbphy->iphy[0].app_rst);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Get phy0 reset control err: %d!\n", ret);
> + return ret;
> + }
> +
> + cbphy->iphy[1].app_rst = devm_reset_control_get_optional(dev, "iphy1");
> + if (IS_ERR(cbphy->iphy[1].app_rst)) {
> + ret = PTR_ERR(cbphy->iphy[1].app_rst);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "Get phy1 reset control err: %d!\n", ret);
> + return ret;
> + }
> +
> + cbphy->app_base = devm_platform_ioremap_resource_byname(pdev, "app");
> + if (IS_ERR(cbphy->app_base))
> + return PTR_ERR(cbphy->app_base);
> +
> + cbphy->cr_base = devm_platform_ioremap_resource_byname(pdev, "core");
> + if (IS_ERR(cbphy->cr_base))
> + return PTR_ERR(cbphy->cr_base);
> +
> + ret = fwnode_property_get_reference_args(fwnode, "intel,syscfg", NULL,
> + 1, 0, &ref);
> + if (ret < 0)
> + return ret;
> +
> + cbphy->id = ref.args[0];
> + cbphy->syscfg = device_node_to_regmap(to_of_node(ref.fwnode));
> + fwnode_handle_put(ref.fwnode);
> +
> + ret = fwnode_property_get_reference_args(fwnode, "intel,hsio", NULL, 1,
> + 0, &ref);
> + if (ret < 0)
> + return ret;
> +
> + cbphy->bid = ref.args[0];
> + cbphy->hsiocfg = device_node_to_regmap(to_of_node(ref.fwnode));
> + fwnode_handle_put(ref.fwnode);
> +
> + ret = fwnode_property_read_u32_array(fwnode, "intel,phy-mode", &val, 1);
> + if (ret)
> + return ret;
> +
> + switch (val) {
> + case PHY_TYPE_PCIE:
> + cbphy->phy_mode = PHY_PCIE_MODE;
> + break;
> +
> + case PHY_TYPE_SATA:
> + cbphy->phy_mode = PHY_SATA_MODE;
> + break;
> +
> + case PHY_TYPE_XPCS:
> + cbphy->phy_mode = PHY_XPCS_MODE;
> + break;
> +
> + default:
> + dev_err(dev, "Invalid PHY mode: %u\n", val);
> + return -EINVAL;
> + }
> +
> + cbphy->clk_rate = intel_iphy_clk_rates[cbphy->phy_mode];
> +
> + if (fwnode_property_present(fwnode, "intel,aggregation"))
> + cbphy->aggr_mode = PHY_DL_MODE;
> + else
> + cbphy->aggr_mode = PHY_SL_MODE;
> +
> + return 0;
> +}
> +
> +static const struct phy_ops intel_cbphy_ops = {
> + .init = intel_cbphy_init,
> + .exit = intel_cbphy_exit,
> + .calibrate = intel_cbphy_calibrate,
> + .owner = THIS_MODULE,
> +};
> +
> +static struct phy *intel_cbphy_xlate(struct device *dev,
> + struct of_phandle_args *args)
> +{
> + struct intel_combo_phy *cbphy = dev_get_drvdata(dev);
> + u32 iphy_id;
> +
> + if (args->args_count < 1) {
> + dev_err(dev, "Invalid number of arguments\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + iphy_id = args->args[0];
> + if (iphy_id >= PHY_MAX_NUM) {
> + dev_err(dev, "Invalid phy instance %d\n", iphy_id);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (cbphy->aggr_mode == PHY_DL_MODE && iphy_id == PHY_1) {
> + dev_err(dev, "Invalid. ComboPhy is in Dual lane mode %d\n", iphy_id);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return cbphy->iphy[iphy_id].phy;
> +}
> +
> +static int intel_cbphy_create(struct intel_combo_phy *cbphy)
> +{
> + struct phy_provider *phy_provider;
> + struct device *dev = cbphy->dev;
> + struct intel_cbphy_iphy *iphy;
> + int i;
> +
> + for (i = 0; i < PHY_MAX_NUM; i++) {
> + iphy = &cbphy->iphy[i];
> + iphy->parent = cbphy;
> + iphy->id = i;
> +
> + /* In dual lane mode skip phy creation for the second phy */
> + if (cbphy->aggr_mode == PHY_DL_MODE && iphy->id == PHY_1)
> + continue;
> +
> + iphy->phy = devm_phy_create(dev, NULL, &intel_cbphy_ops);
> + if (IS_ERR(iphy->phy)) {
> + dev_err(dev, "PHY[%u:%u]: create PHY instance failed!\n",
> + COMBO_PHY_ID(iphy), PHY_ID(iphy));
> +
> + return PTR_ERR(iphy->phy);
> + }
> +
> + phy_set_drvdata(iphy->phy, iphy);
> + }
> +
> + dev_set_drvdata(dev, cbphy);
> + phy_provider = devm_of_phy_provider_register(dev, intel_cbphy_xlate);
> + if (IS_ERR(phy_provider))
> + dev_err(dev, "Register PHY provider failed!\n");
> +
> + return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static int intel_cbphy_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct intel_combo_phy *cbphy;
> + int ret;
> +
> + cbphy = devm_kzalloc(dev, sizeof(*cbphy), GFP_KERNEL);
> + if (!cbphy)
> + return -ENOMEM;
> +
> + cbphy->dev = dev;
> + cbphy->init_cnt = 0;
> + mutex_init(&cbphy->lock);
> + ret = intel_cbphy_fwnode_parse(cbphy);
> + if (ret)
> + return ret;
> +
> + platform_set_drvdata(pdev, cbphy);
> +
> + return intel_cbphy_create(cbphy);
> +}
> +
> +static int intel_cbphy_remove(struct platform_device *pdev)
> +{
> + struct intel_combo_phy *cbphy = platform_get_drvdata(pdev);
> +
> + intel_cbphy_rst_assert(cbphy);
> + clk_disable_unprepare(cbphy->core_clk);
> + return 0;
> +}
> +
> +static const struct of_device_id of_intel_cbphy_match[] = {
> + { .compatible = "intel,combo-phy" },
> + { .compatible = "intel,combophy-lgm" },
> + {}
> +};
> +
> +static struct platform_driver intel_cbphy_driver = {
> + .probe = intel_cbphy_probe,
> + .remove = intel_cbphy_remove,
> + .driver = {
> + .name = "intel-combo-phy",
> + .of_match_table = of_intel_cbphy_match,
> + }
> +};
> +
> +module_platform_driver(intel_cbphy_driver);
> +
> +MODULE_DESCRIPTION("Intel Combo-phy driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.11.0
--
~Vinod
^ permalink raw reply
* Re: [PATCH v7 01/12] dt-bindings: add img,pvrsgx.yaml for Imagination GPUs
From: H. Nikolaus Schaller @ 2020-05-15 7:18 UTC (permalink / raw)
To: Paul Cercueil, Paul Burton
Cc: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland,
Benoît Cousson, Tony Lindgren, Ralf Baechle, James Hogan,
Kukjin Kim, Krzysztof Kozlowski, Maxime Ripard, Chen-Yu Tsai,
Thomas Bogendoerfer, Jonathan Bakker, Philipp Rossak,
open list:DRM PANEL DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux Kernel Mailing List, linux-omap,
OpenPVRSGX Linux Driver Group, Discussions about the Letux Kernel,
kernel, linux-mips, arm-soc, linux-samsung-soc
In-Reply-To: <08B861A8-D4C2-48A6-9B05-B8CA43312834@goldelico.com>
Hi Paul & Paul,
> Am 03.05.2020 um 18:41 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>
> Hi Paul and Paul,
>
>> Am 03.05.2020 um 16:18 schrieb Paul Cercueil <paul@crapouillou.net>:
>>
>>
>>
>> Le dim. 3 mai 2020 à 15:31, H. Nikolaus Schaller <hns@goldelico.com> a écrit :
>>> Hi Paul,
>>>> Am 03.05.2020 um 14:52 schrieb Paul Cercueil <paul@crapouillou.net>:
>>>>>> It's possible to forbid the presence of the 'clocks' property on some implementations, and require it on others.
>>>>> To be precise we have to specify the exact number of clocks (between 0 and 4) for every architecture.
>>>>> This also contradicts my dream to get rid of the architecture specific components in the long run. My dream (because I can't tell how it can be done) is that we can one day develop something which just needs compatible = img,530 or imp,540 or img,544. Then we can't make the number clocks depend on the implementation any more.
>>>> As we said before, the number of clocks is a property of the GPU and *not* its integration into the SoC.
>>> Well, it is a not very well documented property of the GPU. We have no data sheet of the standalone GPU. Only several SoC data sheets which give some indications.
>>
>> Maybe we can nicely ask them?
>
> There is some (old) answer here:
>
> https://github.com/MIPS/CI20_linux/blob/ci20-v3.18/arch/mips/boot/dts/jz4780.dtsi#L63
>
>> I expect Paul Burton to have some contacts at ImgTec. Asking for a doc would be too much, but maybe they can help a bit with the DT bindings.
>
> Good idea! It is definitively worth to try. Therefore I have moved him from CC: to To:
Do we already have an idea if we can get into contact and get help from ImgTec for this topic or if we have to live with what we have?
BR and thanks,
Nikolaus
^ permalink raw reply
* Re: [PATCH v3 10/15] net: ethernet: mtk-eth-mac: new driver
From: Bartosz Golaszewski @ 2020-05-15 7:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jonathan Corbet, Rob Herring, David S . Miller, Matthias Brugger,
John Crispin, Sean Wang, Mark Lee, Jakub Kicinski, Fabien Parent,
Heiner Kallweit, Edwin Peer, DTML, linux-kernel@vger.kernel.org,
Networking, Linux ARM, moderated list:ARM/Mediatek SoC...,
Stephane Le Provost, Pedro Tsai, Andrew Perepech,
Bartosz Golaszewski
In-Reply-To: <CAK8P3a3=xgbvqrSpCK5h96eRH32AA7xnoK2ossvT0-cLFLzmXA@mail.gmail.com>
czw., 14 maj 2020 o 18:19 Arnd Bergmann <arnd@arndb.de> napisał(a):
>
> On Thu, May 14, 2020 at 10:00 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > This adds the driver for the MediaTek Ethernet MAC used on the MT8* SoC
> > family. For now we only support full-duplex.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Looks very nice overall. Just a few things I noticed, and some ideas
> that may or may not make sense:
>
> > +/* This is defined to 0 on arm64 in arch/arm64/include/asm/processor.h but
> > + * this IP doesn't work without this alignment being equal to 2.
> > + */
> > +#ifdef NET_IP_ALIGN
> > +#undef NET_IP_ALIGN
> > +#endif
> > +#define NET_IP_ALIGN 2
>
> Maybe you should just define your own macro instead of replacing
> the normal one then?
>
I did in an earlier version and was told to use NET_IP_ALIGN but then
found out its value on arm64 doesn't work for me so I did the thing
that won't make anybody happy - redefine the existing constant. :)
> > +static void mtk_mac_lock(struct mtk_mac_priv *priv)
> > +{
> > + spin_lock_irqsave(&priv->lock, priv->lock_flags);
> > +}
> > +
> > +static void mtk_mac_unlock(struct mtk_mac_priv *priv)
> > +{
> > + spin_unlock_irqrestore(&priv->lock, priv->lock_flags);
> > +}
>
> This looks wrong: you should not have shared 'flags' passed into
> spin_lock_irqsave(), and I don't even see a need to use the
> irqsave variant of the lock in the first place.
>
> Maybe start by open-coding the lock and remove the wrappers
> above.
>
> Then see if you can use a cheaper spin_lock_bh() or plain spin_lock()
> instead of irqsave.
>
This is from an earlier version where I did a lot more in hard irq
context. Now that almost all of the processing happens in soft-irq
context I guess you're right - I can go with a regular spin_lock().
> Finally, see if this can be done in a lockless way by relying on
> appropriate barriers and separating the writers into separate
> cache lines. From a brief look at the driver I think it can be done
> without too much trouble.
>
Unfortunately I do need some locking. Accessing RX and TX descriptors
at the same time seems to upset the controller. I experimented a lot
with barriers but it turned out that I got a lot of weird bugs at high
throughput.
> > +static unsigned int mtk_mac_intr_read_and_clear(struct mtk_mac_priv *priv)
> > +{
> > + unsigned int val;
> > +
> > + regmap_read(priv->regs, MTK_MAC_REG_INT_STS, &val);
> > + regmap_write(priv->regs, MTK_MAC_REG_INT_STS, val);
> > +
> > + return val;
> > +}
>
> Do you actually need to read the register? That is usually a relatively
> expensive operation, so if possible try to use clear the bits when
> you don't care which bits were set.
>
I do care, I'm afraid. The returned value is being used in the napi
poll callback to see which ring to process.
> > +/* All processing for TX and RX happens in the napi poll callback. */
> > +static irqreturn_t mtk_mac_handle_irq(int irq, void *data)
> > +{
> > + struct mtk_mac_priv *priv;
> > + struct net_device *ndev;
> > +
> > + ndev = data;
> > + priv = netdev_priv(ndev);
> > +
> > + if (netif_running(ndev)) {
> > + mtk_mac_intr_mask_all(priv);
> > + napi_schedule(&priv->napi);
> > + }
> > +
> > + return IRQ_HANDLED;
>
>
> > +static int mtk_mac_netdev_start_xmit(struct sk_buff *skb,
> > + struct net_device *ndev)
> > +{
> > + struct mtk_mac_priv *priv = netdev_priv(ndev);
> > + struct mtk_mac_ring *ring = &priv->tx_ring;
> > + struct device *dev = mtk_mac_get_dev(priv);
> > + struct mtk_mac_ring_desc_data desc_data;
> > +
> > + desc_data.dma_addr = mtk_mac_dma_map_tx(priv, skb);
> > + if (dma_mapping_error(dev, desc_data.dma_addr))
> > + goto err_drop_packet;
> > +
> > + desc_data.skb = skb;
> > + desc_data.len = skb->len;
> > +
> > + mtk_mac_lock(priv);
> > + mtk_mac_ring_push_head_tx(ring, &desc_data);
> > +
> > + if (mtk_mac_ring_full(ring))
> > + netif_stop_queue(ndev);
> > + mtk_mac_unlock(priv);
> > +
> > + mtk_mac_dma_resume_tx(priv);
> > +
> > + return NETDEV_TX_OK;
> > +
> > +err_drop_packet:
> > + dev_kfree_skb(skb);
> > + ndev->stats.tx_dropped++;
> > + return NETDEV_TX_BUSY;
> > +}
>
> I would always add BQL flow control in new drivers, using
> netdev_sent_queue here...
>
Ok, will do.
> > +static int mtk_mac_tx_complete_one(struct mtk_mac_priv *priv)
> > +{
> > + struct mtk_mac_ring *ring = &priv->tx_ring;
> > + struct mtk_mac_ring_desc_data desc_data;
> > + int ret;
> > +
> > + ret = mtk_mac_ring_pop_tail(ring, &desc_data);
> > + if (ret)
> > + return ret;
> > +
> > + mtk_mac_dma_unmap_tx(priv, &desc_data);
> > + dev_kfree_skb_irq(desc_data.skb);
> > +
> > + return 0;
> > +}
>
> ... and netdev_completed_queue() here.
>
Same here.
> > +static void mtk_mac_tx_complete_all(struct mtk_mac_priv *priv)
> > +{
> > + struct mtk_mac_ring *ring = &priv->tx_ring;
> > + struct net_device *ndev = priv->ndev;
> > + int ret;
> > +
> > + for (;;) {
> > + mtk_mac_lock(priv);
> > +
> > + if (!mtk_mac_ring_descs_available(ring)) {
> > + mtk_mac_unlock(priv);
> > + break;
> > + }
> > +
> > + ret = mtk_mac_tx_complete_one(priv);
> > + if (ret) {
> > + mtk_mac_unlock(priv);
> > + break;
> > + }
> > +
> > + if (netif_queue_stopped(ndev))
> > + netif_wake_queue(ndev);
> > +
> > + mtk_mac_unlock(priv);
> > + }
> > +}
>
> It looks like most of the stuff inside of the loop can be pulled out
> and only done once here.
>
I did that in one of the previous submissions but it was pointed out
to me that a parallel TX path may fill up the queue before I wake it.
> > +static int mtk_mac_poll(struct napi_struct *napi, int budget)
> > +{
> > + struct mtk_mac_priv *priv;
> > + unsigned int status;
> > + int received = 0;
> > +
> > + priv = container_of(napi, struct mtk_mac_priv, napi);
> > +
> > + status = mtk_mac_intr_read_and_clear(priv);
> > +
> > + /* Clean up TX */
> > + if (status & MTK_MAC_BIT_INT_STS_TNTC)
> > + mtk_mac_tx_complete_all(priv);
> > +
> > + /* Receive up to $budget packets */
> > + if (status & MTK_MAC_BIT_INT_STS_FNRC)
> > + received = mtk_mac_process_rx(priv, budget);
> > +
> > + /* One of the counter reached 0x8000000 - update stats and reset all
> > + * counters.
> > + */
> > + if (status & MTK_MAC_REG_INT_STS_MIB_CNT_TH) {
> > + mtk_mac_update_stats(priv);
> > + mtk_mac_reset_counters(priv);
> > + }
> > +
> > + if (received < budget)
> > + napi_complete_done(napi, received);
> > +
> > + mtk_mac_intr_unmask_all(priv);
> > +
> > + return received;
> > +}
>
> I think you want to leave (at least some of) the interrupts masked
> if your budget is exhausted, to avoid generating unnecessary
> irqs.
>
The networking stack shouldn't queue any new TX packets if the queue
is stopped - is this really worth complicating the code? Looks like
premature optimization IMO.
> It may also be faster to not mask/unmask at all but just
> clear the interrupts that you have finished processing
>
Bart
^ permalink raw reply
* Re: [PATCH v7 0/2] Amlogic 32-bit Meson SoC SDHC MMC controller driver
From: Ulf Hansson @ 2020-05-15 7:09 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: open list:ARM/Amlogic Meson..., linux-mmc@vger.kernel.org,
Rob Herring, DTML, Jianxin Pan, Linux Kernel Mailing List,
yinxin_1989, Linux ARM, lnykww, Anand Moon, Jerome Brunet
In-Reply-To: <20200512204147.504087-1-martin.blumenstingl@googlemail.com>
On Tue, 12 May 2020 at 22:42, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hello,
>
> this is the patchset for a driver for the Amlogic "SDHC" MMC controller
> found on Meson6, Meson8, Meson8b and Meson8m2 SoCs.
>
> The public S805 (Meson8b) datasheet has some documentation starting on
> page 74: [0]
>
> It's performance is still not as good as the driver from Amlogic's 3.10
> kernel, but it does not corrupt data anymore (as RFC v1 did).
>
> Special thanks to the people who supported me off-list - you are
> amazing and deserve to be mentioned here:
> - Xin Yin who helped me fix two more write corruption problems. I am
> hoping that he will reply with Reviewed-by, Tested-by and Bug-fixed-by
> - Jianxin Pan for sharing some of the internal workings of this MMC
> controller with me
> - Wei Wang for spotting the initial write corruption problem and helping
> test this driver on his board. I have his permission to add his
> Tested-by (off-list, he's Cc'ed so if there's any problem he can speak
> up)
>
>
> Changes since v6 at [6]:
> - both patches: dropped the clock #include from the dt-bindings as well
> as #clock-cells = <1> and the self-referencing clock inputs. Instead
> the driver will not be registering a clock provider anymore. The
> clock references are obtained using "clk_hw.clk" (thus not going
> through the dt-bindings anymore) as suggested in [7] until a better
> solution is implemented. A TODO comment is also in place so it's
> easier to find this temporary workaround when the new helper exists.
> - dropped Rob's Reviewed-by because I modified the dt-bindings. schema
> validation still passes on my build machine.
> - patch #2: dropped MMC_CAP_ERASE due to the following patch which is
> queued in mmc's -next: "mmc: host: Drop redundant MMC_CAP_ERASE"
> - patch #2: fill all clk_{mux,divider,gate,hw} values in
> meson_mx_sdhc_register_clkc instead of using loops and two separate
> structs to make the code easier to read. Thanks to Jerome for the
> suggestion.
> - I decided to keep all the Tested-by's because testing was smooth
> for me and none of the clock calculation formulas has changed (only
> the API how to obtain the clocks).
>
> Changes since v5 at [5] (thanks to Ulf and Jerome for the feedback):
> - changed copyright year to 2020
> - move register #defines to a separate header file
> - drop unused include linux/clk-provider.h from meson-mx-sdhc.c
> - used #defines for regmap_read_poll_timeout timeout/sleep values
> - set MMC_CAP_WAIT_WHILE_BUSY
> - move the clock controller code to mmc/host/meson-mx-sdhc-clkc.c and
> don't register a separate platform_device for it. This also means
> that the driver switched from clk_regmap to the standard
> clk_{divider,gate,mux}_ops
> - dropped ".index = -1" for clk_parent_data with .fw_name
> - use CLK_SET_RATE_PARENT on all leaf clocks and drop
> CLK_SET_RATE_GATE
> - switch from parent_data.name to parent_hws
> - use fallthrough; instead of fallthrough comment
> - added Anand's Tested-by - thank you!
> - I decided to keep all Tested-by and Reviewed-by because they were
> only for the MMC controller part and I have barely touched that with
> this update.
> - Ulf asked if the timeout can be shortened. I believe it can but I
> have no documentation for it. Thus I need a test-case to see if my
> assumptions are correct - thus I have not addressed this in v6 yet
>
> Changes since v4 at [4]:
> - move the four clkin clock inputs to the start of the clock-names list
> as suggested by Rob, affects patch #1
> - fixed #include statement in dt-bindings example in patch #1
>
> Changes since v3 at [3]:
> - split the clock bits into a separate clock controller driver because
> of two reasons: 1) it keeps the MMC controller driver mostly clean of
> the clock bits 2) the pure clock controller can use
> devm_clk_hw_register() (instead of devm_clk_register(), which is
> deprecated) and the MMC controller can act as a pure clock consumer.
> This also affects the dt-bindings which is why I dropped Rob's
> Reviewed-by. Thanks to Ulf for the suggestions
>
> Changes since v2 at [2]:
> - rebased on top of v5.5-rc1
> - added Rob's and Xin Yin's Reviewed-by and Tested-by (thank you!)
> - (note: Kevin had v2 of this series in -next for a few days so the
> build test robots could play with it. I haven't received any negative
> feedback in that time)
>
> Changes since RFC v1 at [1]:
> - don't set MESON_SDHC_MISC_MANUAL_STOP to fix one of three write
> corruption problems. the out-of-tree 3.10 "reference" driver doesn't
> set it either
> - check against data->flags instead of cmd->flags when testing for
> MMC_DATA_WRITE as spotted by Xin Yin (many thanks!). This fixes
> another write corruption problem
> - clear the FIFOs after successfully transferring data as suggested by
> Xin Yin (many thanks!). This is what the 3.10 driver did and fixes yet
> another write corruption problem
> - integrate the clock suggestions from Jianxin Pan so the driver is now
> able to set up the clocks correctly for all known cases. documentation
> is also added to the patch description. Thank you Jianxin for the
> help!
> - set the correct max_busy_timeout as suggested by Jianxin Pan (thanks!)
> - convert the dt-bindings to .yaml (which is why I didn't add Rob's
> Reviewed-by)
> - switch to struct clk_parent_data as part of newer common clock
> framework APIs to simplify the clock setup
> - dropped CMD23 support because it seems to hurt read and write
> performance by 10-20% in my tests. it's not clear why, but for now we
> can live without this.
> - use devm_platform_ioremap_resource instead of open-coding it
>
>
> [0] https://dn.odroid.com/S805/Datasheet/S805_Datasheet%20V0.8%2020150126.pdf
> [1] https://patchwork.kernel.org/cover/11035505/
> [2] http://lists.infradead.org/pipermail/linux-amlogic/2019-November/014576.html
> [3] https://patchwork.kernel.org/cover/11283179/
> [4] https://patchwork.kernel.org/cover/11329017/
> [5] https://patchwork.kernel.org/cover/11463341/
> [6] https://patchwork.kernel.org/cover/11515603/
> [7] https://lore.kernel.org/linux-clk/158870581453.26370.15255632521260524214@swboyd.mtv.corp.google.com/
>
>
> Martin Blumenstingl (2):
> dt-bindings: mmc: Document the Amlogic Meson SDHC MMC host controller
> mmc: host: meson-mx-sdhc: new driver for the Amlogic Meson SDHC host
>
> .../bindings/mmc/amlogic,meson-mx-sdhc.yaml | 68 ++
> drivers/mmc/host/Kconfig | 14 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/meson-mx-sdhc-clkc.c | 158 +++
> drivers/mmc/host/meson-mx-sdhc.c | 907 ++++++++++++++++++
> drivers/mmc/host/meson-mx-sdhc.h | 141 +++
> 6 files changed, 1289 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/amlogic,meson-mx-sdhc.yaml
> create mode 100644 drivers/mmc/host/meson-mx-sdhc-clkc.c
> create mode 100644 drivers/mmc/host/meson-mx-sdhc.c
> create mode 100644 drivers/mmc/host/meson-mx-sdhc.h
>
Applied for next, thanks!
Kind regards
Uffe
^ permalink raw reply
* [V5 PATCH 1/2] dt-bindings: Added device tree binding for max98390
From: Steve Lee @ 2020-05-15 7:07 UTC (permalink / raw)
To: lgirdwood, broonie, robh+dt, alsa-devel, devicetree, linux-kernel
Cc: ryan.lee.maxim, ryans.lee, steves.lee, steves.lee.maxim
Add documentation for DT binding of max98390 amplifier driver.
Signed-off-by: Steve Lee <steves.lee@maximintegrated.com>
---
Changed since V4:
* No changes.
Changed since V3:
* No changes.
Changed since V2:
* No changes.
Changed since V1:
* Modified sample text in example
.../devicetree/bindings/sound/max98390.txt | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/max98390.txt
diff --git a/Documentation/devicetree/bindings/sound/max98390.txt b/Documentation/devicetree/bindings/sound/max98390.txt
new file mode 100644
index 000000000000..0ddd4c6ae55e
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/max98390.txt
@@ -0,0 +1,26 @@
+Maxim Integrated MAX98390 Speaker Amplifier
+
+This device supports I2C.
+
+Required properties:
+
+ - compatible : "maxim,max98390"
+
+ - reg : the I2C address of the device.
+
+Optional properties:
+
+- maxim,temperature_calib
+ u32. The calculated temperature data was measured while doing the calibration. Data : Temp / 100 * 2^12
+
+- maxim,r0_calib
+ u32. This is r0 calibration data which was measured in factory mode.
+
+Example:
+
+codec: max98390@38 {
+ compatible = "maxim,max98390";
+ reg = <0x38>;
+ maxim,temperature_calib = <1024>;
+ maxim,r0_calib = <100232>;
+};
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] dt-bindings: hisilicon: Add CTI bindings for hi-6220
From: Wei Xu @ 2020-05-15 7:06 UTC (permalink / raw)
To: Mike Leach, devicetree, linux-arm-kernel, coresight
Cc: mathieu.poirier, suzuki.poulose
In-Reply-To: <20200415201259.15831-1-mike.leach@linaro.org>
Hi Mike,
On 2020/4/16 4:12, Mike Leach wrote:
> Adds in CTI device tree information for the Hikey620 board.
>
> Tested on Linux 5.7-rc1.
>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> Tested-by: Leo Yan <leo.yan@linaro.org>
Thanks!
Applied to the hisilicon arm64 dt tree.
Best Regards,
Wei
> ---
> .../boot/dts/hisilicon/hi6220-coresight.dtsi | 130 ++++++++++++++++--
> 1 file changed, 122 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi
> index 651771a73ed6..27f067e87601 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220-coresight.dtsi
> @@ -213,7 +213,7 @@
> };
> };
>
> - etm@f659c000 {
> + etm0: etm@f659c000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0 0xf659c000 0 0x1000>;
>
> @@ -232,7 +232,7 @@
> };
> };
>
> - etm@f659d000 {
> + etm1: etm@f659d000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0 0xf659d000 0 0x1000>;
>
> @@ -251,7 +251,7 @@
> };
> };
>
> - etm@f659e000 {
> + etm2: etm@f659e000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0 0xf659e000 0 0x1000>;
>
> @@ -270,7 +270,7 @@
> };
> };
>
> - etm@f659f000 {
> + etm3: etm@f659f000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0 0xf659f000 0 0x1000>;
>
> @@ -289,7 +289,7 @@
> };
> };
>
> - etm@f65dc000 {
> + etm4: etm@f65dc000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0 0xf65dc000 0 0x1000>;
>
> @@ -308,7 +308,7 @@
> };
> };
>
> - etm@f65dd000 {
> + etm5: etm@f65dd000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0 0xf65dd000 0 0x1000>;
>
> @@ -327,7 +327,7 @@
> };
> };
>
> - etm@f65de000 {
> + etm6: etm@f65de000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0 0xf65de000 0 0x1000>;
>
> @@ -346,7 +346,7 @@
> };
> };
>
> - etm@f65df000 {
> + etm7: etm@f65df000 {
> compatible = "arm,coresight-etm4x", "arm,primecell";
> reg = <0 0xf65df000 0 0x1000>;
>
> @@ -364,5 +364,119 @@
> };
> };
> };
> +
> + /* System CTIs */
> + /* CTI 0 - TMC and TPIU connections */
> + cti@f6403000 {
> + compatible = "arm,coresight-cti", "arm,primecell";
> + reg = <0 0xf6403000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> + };
> +
> + /* CTI - CPU-0 */
> + cti@f6598000 {
> + compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> + "arm,primecell";
> + reg = <0 0xf6598000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> +
> + cpu = <&cpu0>;
> + arm,cs-dev-assoc = <&etm0>;
> + };
> +
> + /* CTI - CPU-1 */
> + cti@f6599000 {
> + compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> + "arm,primecell";
> + reg = <0 0xf6599000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> +
> + cpu = <&cpu1>;
> + arm,cs-dev-assoc = <&etm1>;
> + };
> +
> + /* CTI - CPU-2 */
> + cti@f659a000 {
> + compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> + "arm,primecell";
> + reg = <0 0xf659a000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> +
> + cpu = <&cpu2>;
> + arm,cs-dev-assoc = <&etm2>;
> + };
> +
> + /* CTI - CPU-3 */
> + cti@f659b000 {
> + compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> + "arm,primecell";
> + reg = <0 0xf659b000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> +
> + cpu = <&cpu3>;
> + arm,cs-dev-assoc = <&etm3>;
> + };
> +
> + /* CTI - CPU-4 */
> + cti@f65d8000 {
> + compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> + "arm,primecell";
> + reg = <0 0xf65d8000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> +
> + cpu = <&cpu4>;
> + arm,cs-dev-assoc = <&etm4>;
> + };
> +
> + /* CTI - CPU-5 */
> + cti@f65d9000 {
> + compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> + "arm,primecell";
> + reg = <0 0xf65d9000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> +
> + cpu = <&cpu5>;
> + arm,cs-dev-assoc = <&etm5>;
> + };
> +
> + /* CTI - CPU-6 */
> + cti@f65da000 {
> + compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> + "arm,primecell";
> + reg = <0 0xf65da000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> +
> + cpu = <&cpu6>;
> + arm,cs-dev-assoc = <&etm6>;
> + };
> +
> + /* CTI - CPU-7 */
> + cti@f65db000 {
> + compatible = "arm,coresight-cti-v8-arch", "arm,coresight-cti",
> + "arm,primecell";
> + reg = <0 0xf65db000 0 0x1000>;
> +
> + clocks = <&acpu_sctrl HI6220_ACPU_SFT_AT_S>;
> + clock-names = "apb_pclk";
> +
> + cpu = <&cpu7>;
> + arm,cs-dev-assoc = <&etm7>;
> + };
> };
> };
>
^ permalink raw reply
* Re: [PATCH 5/5] PCI: uniphier: Add error message when failed to get phy
From: kbuild test robot @ 2020-05-15 6:51 UTC (permalink / raw)
To: Kunihiko Hayashi, Bjorn Helgaas, Lorenzo Pieralisi, Jingoo Han,
Gustavo Pimentel, Rob Herring, Masahiro Yamada
Cc: kbuild-all, linux-pci, devicetree, linux-arm-kernel, linux-kernel,
Masami Hiramatsu, Jassi Brar, Kunihiko Hayashi
In-Reply-To: <1589518105-18368-6-git-send-email-hayashi.kunihiko@socionext.com>
[-- Attachment #1: Type: text/plain, Size: 3606 bytes --]
Hi Kunihiko,
I love your patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[also build test WARNING on robh/for-next v5.7-rc5 next-20200514]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Kunihiko-Hayashi/PCI-uniphier-Add-features-for-UniPhier-PCIe-host-controller/20200515-125031
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from include/linux/device.h:15:0,
from include/linux/pci.h:37,
from drivers/pci/controller/dwc/pcie-uniphier.c:18:
drivers/pci/controller/dwc/pcie-uniphier.c: In function 'uniphier_pcie_probe':
>> drivers/pci/controller/dwc/pcie-uniphier.c:470:16: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
dev_err(dev, "Failed to get phy (%d)n", PTR_ERR(priv->phy));
^
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
#define dev_fmt(fmt) fmt
^~~
>> drivers/pci/controller/dwc/pcie-uniphier.c:470:3: note: in expansion of macro 'dev_err'
dev_err(dev, "Failed to get phy (%d)n", PTR_ERR(priv->phy));
^~~~~~~
vim +470 drivers/pci/controller/dwc/pcie-uniphier.c
430
431 static int uniphier_pcie_probe(struct platform_device *pdev)
432 {
433 struct device *dev = &pdev->dev;
434 struct uniphier_pcie_priv *priv;
435 struct resource *res;
436 int ret;
437
438 priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
439 if (!priv)
440 return -ENOMEM;
441
442 priv->pci.dev = dev;
443 priv->pci.ops = &dw_pcie_ops;
444
445 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
446 priv->pci.dbi_base = devm_pci_remap_cfg_resource(dev, res);
447 if (IS_ERR(priv->pci.dbi_base))
448 return PTR_ERR(priv->pci.dbi_base);
449
450 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");
451 priv->pci.atu_base = devm_pci_remap_cfg_resource(dev, res);
452 if (IS_ERR(priv->pci.atu_base))
453 priv->pci.atu_base = NULL;
454
455 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "link");
456 priv->base = devm_ioremap_resource(dev, res);
457 if (IS_ERR(priv->base))
458 return PTR_ERR(priv->base);
459
460 priv->clk = devm_clk_get(dev, NULL);
461 if (IS_ERR(priv->clk))
462 return PTR_ERR(priv->clk);
463
464 priv->rst = devm_reset_control_get_shared(dev, NULL);
465 if (IS_ERR(priv->rst))
466 return PTR_ERR(priv->rst);
467
468 priv->phy = devm_phy_optional_get(dev, "pcie-phy");
469 if (IS_ERR(priv->phy)) {
> 470 dev_err(dev, "Failed to get phy (%d)\n", PTR_ERR(priv->phy));
471 return PTR_ERR(priv->phy);
472 }
473
474 platform_set_drvdata(pdev, priv);
475
476 ret = uniphier_pcie_host_enable(priv);
477 if (ret)
478 return ret;
479
480 return uniphier_add_pcie_port(priv, pdev);
481 }
482
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 72245 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/6] dt-bindings: mfd: add Khadas Microcontroller bindings
From: Amit Kucheria @ 2020-05-15 6:42 UTC (permalink / raw)
To: Neil Armstrong
Cc: Lee Jones, Zhang Rui, Daniel Lezcano, Srini Kandagatla,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-amlogic, Linux PM list, lakml, LKML, Rob Herring
In-Reply-To: <20200512132613.31507-2-narmstrong@baylibre.com>
On Tue, May 12, 2020 at 6:56 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This Microcontroller is present on the Khadas VIM1, VIM2, VIM3 and Edge
> boards.
>
> It has multiple boot control features like password check, power-on
> options, power-off control and system FAN control on recent boards.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
> .../devicetree/bindings/mfd/khadas,mcu.yaml | 44 +++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
> new file mode 100644
> index 000000000000..a3b976f101e8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/khadas,mcu.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/khadas,mcu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Khadas on-board Microcontroller Device Tree Bindings
> +
> +maintainers:
> + - Neil Armstrong <narmstrong@baylibre.com>
> +
> +description: |
> + Khadas embeds a microcontroller on their VIM and Edge boards adding some
> + system feature as PWM Fan control (for VIM2 rev14 or VIM3), User memory
> + storage, IR/Key resume control, system power LED control and more.
> +
> +properties:
> + compatible:
> + enum:
> + - khadas,mcu # MCU revision is discoverable
> +
> + "#cooling-cells": # Only needed for boards having FAN control feature
> + const: 2
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + khadas_mcu: system-controller@18 {
> + compatible = "khadas,mcu";
> + reg = <0x18>;
> + #cooling-cells = <2>;
> + };
> + };
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH v2 5/6] dmaengine: dw: Introduce max burst length hw config
From: Vinod Koul @ 2020-05-15 6:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Serge Semin, Viresh Kumar, Dan Williams,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Rob Herring, linux-mips, devicetree, dmaengine,
linux-kernel
In-Reply-To: <20200512191208.GG185537@smile.fi.intel.com>
On 12-05-20, 22:12, Andy Shevchenko wrote:
> On Tue, May 12, 2020 at 05:08:20PM +0300, Serge Semin wrote:
> > On Fri, May 08, 2020 at 02:41:53PM +0300, Andy Shevchenko wrote:
> > > On Fri, May 08, 2020 at 01:53:03PM +0300, Serge Semin wrote:
> > > > IP core of the DW DMA controller may be synthesized with different
> > > > max burst length of the transfers per each channel. According to Synopsis
> > > > having the fixed maximum burst transactions length may provide some
> > > > performance gain. At the same time setting up the source and destination
> > > > multi size exceeding the max burst length limitation may cause a serious
> > > > problems. In our case the system just hangs up. In order to fix this
> > > > lets introduce the max burst length platform config of the DW DMA
> > > > controller device and don't let the DMA channels configuration code
> > > > exceed the burst length hardware limitation. Depending on the IP core
> > > > configuration the maximum value can vary from channel to channel.
> > > > It can be detected either in runtime from the DWC parameter registers
> > > > or from the dedicated dts property.
> > >
> > > I'm wondering what can be the scenario when your peripheral will ask something
> > > which is not supported by DMA controller?
> >
> > I may misunderstood your statement, because seeing your activity around my
> > patchsets including the SPI patchset and sometimes very helpful comments,
> > this question answer seems too obvious to see you asking it.
> >
> > No need to go far for an example. See the DW APB SSI driver. Its DMA module
> > specifies the burst length to be 16, while not all of ours channels supports it.
> > Yes, originally it has been developed for the Intel Midfield SPI, but since I
> > converted the driver into a generic code we can't use a fixed value. For instance
> > in our hardware only two DMA channels of total 16 are capable of bursting up to
> > 16 bytes (data items) at a time, the rest of them are limited with up to 4 bytes
> > burst length. While there are two SPI interfaces, each of which need to have two
> > DMA channels for communications. So I need four channels in total to allocate to
> > provide the DMA capability for all interfaces. In order to set the SPI controller
> > up with valid optimized parameters the max-burst-length is required. Otherwise we
> > can end up with buffers overrun/underrun.
>
> Right, and we come to the question which channel better to be used by SPI and
> the rest devices. Without specific filter function you can easily get into a
> case of inverted optimizations, when SPI got channels with burst = 4, while
> it's needed 16, and other hardware otherwise. Performance wise it's worse
> scenario which we may avoid in the first place, right?
If one has channels which are different and described as such in DT,
then I think it does make sense to specify in your board-dt about the
specific channels you would require...
>
> > > Peripheral needs to supply a lot of configuration parameters specific to the
> > > DMA controller in use (that's why we have struct dw_dma_slave).
> > > So, seems to me the feasible approach is supply correct data in the first place.
> >
> > How to supply a valid data if clients don't know the DMA controller limitations
> > in general?
>
> This is a good question. DMA controllers are quite different and having unified
> capabilities structure for all is almost impossible task to fulfil. That's why
> custom filter function(s) can help here. Based on compatible string you can
> implement whatever customized quirks like two functions, for example, to try 16
> burst size first and fallback to 4 if none was previously found.
>
> > > If you have specific channels to acquire then you probably need to provide a
> > > custom xlate / filter functions. Because above seems a bit hackish workaround
> > > of dynamic channel allocation mechanism.
> >
> > No, I don't have a specific channel to acquire and in general you may use any
> > returned from the DMA subsystem (though some platforms may need a dedicated
> > channels to use, in this case xlate / filter is required). In our SoC any DW DMAC
> > channel can be used for any DMA-capable peripherals like SPI, I2C, UART. But the
> > their DMA settings must properly and optimally configured. It can be only done
> > if you know the DMA controller parameters like max burst length, max block-size,
> > etc.
> >
> > So no. The change proposed by this patch isn't workaround, but a useful feature,
> > moreover expected to be supported by the generic DMA subsystem.
>
> See above.
>
> > > But let's see what we can do better. Since maximum is defined on the slave side
> > > device, it probably needs to define minimum as well, otherwise it's possible
> > > that some hardware can't cope underrun bursts.
> >
> > There is no need to define minimum if such limit doesn't exists except a
> > natural 1. Moreover it doesn't exist for all DMA controllers seeing noone has
> > added such capability into the generic DMA subsystem so far.
>
> There is a contract between provider and consumer about DMA resource. That's
> why both sides should participate in fulfilling it. Theoretically it may be a
> hardware that doesn't support minimum burst available in DMA by a reason. For
> such we would need minimum to be provided as well.
Agreed and if required caps should be extended to tell consumer the
minimum values supported.
--
~Vinod
^ permalink raw reply
* Re: [PATCH v2 4/6] dmaengine: dw: Print warning if multi-block is unsupported
From: Vinod Koul @ 2020-05-15 6:30 UTC (permalink / raw)
To: Serge Semin
Cc: Andy Shevchenko, Serge Semin, Mark Brown, Viresh Kumar,
Dan Williams, Alexey Malahov, Thomas Bogendoerfer, Paul Burton,
Ralf Baechle, Arnd Bergmann, Rob Herring, linux-mips, devicetree,
dmaengine, Linux Kernel Mailing List
In-Reply-To: <20200512124206.l3uv5hg2zimi24dq@mobilestation>
Hi Serge,
On 12-05-20, 15:42, Serge Semin wrote:
> Vinod,
>
> Could you join the discussion for a little bit?
>
> In order to properly fix the problem discussed in this topic, we need to
> introduce an additional capability exported by DMA channel handlers on per-channel
> basis. It must be a number, which would indicate an upper limitation of the SG list
> entries amount.
> Something like this would do it:
> struct dma_slave_caps {
> ...
> unsigned int max_sg_nents;
> ...
Looking at the discussion, I agree we should can this up in the
interface. The max_dma_len suggests the length of a descriptor allowed,
it does not convey the sg_nents supported which in the case of nollp is
one.
Btw is this is a real hardware issue, I have found that value of such
hardware is very less and people did fix it up in subsequent revs to add
llp support.
Also, another question is why this cannot be handled in driver, I agree
your hardware does not support llp but that does not stop you from
breaking a multi_sg list into N hardware descriptors and keep submitting
them (for this to work submission should be done in isr and not in bh,
unfortunately very few driver take that route). TBH the max_sg_nents or
max_dma_len are HW restrictions and SW *can* deal with then :-)
In an idea world, you should break the sw descriptor submitted into N hw
descriptors and submit to hardware and let user know when the sw
descriptor is completed. Of course we do not do that :(
> };
> As Andy suggested it's value should be interpreted as:
> 0 - unlimited number of entries,
> 1:MAX_UINT - actual limit to the number of entries.
Hmm why 0, why not MAX_UINT for unlimited?
> In addition to that seeing the dma_get_slave_caps() method provide the caps only
> by getting them from the DMA device descriptor, while we need to have an info on
> per-channel basis, it would be good to introduce a new DMA-device callback like:
> struct dma_device {
> ...
> int (*device_caps)(struct dma_chan *chan,
> struct dma_slave_caps *caps);
Do you have a controller where channel caps are on per-channel basis?
> ...
> };
> So the DMA driver could override the generic DMA device capabilities with the
> values specific to the DMA channels. Such functionality will be also helpful for
> the max-burst-len parameter introduced by this patchset, since depending on the
> IP-core synthesis parameters it may be channel-specific.
>
> Alternatively we could just introduce a new fields to the dma_chan structure and
> retrieve the new caps values from them in the dma_get_slave_caps() method.
> Though the solution with callback I like better.
>
> What is your opinion about this? What solution you'd prefer?
>
> On Tue, May 12, 2020 at 12:08:00AM +0300, Andy Shevchenko wrote:
> > On Tue, May 12, 2020 at 12:07:14AM +0300, Andy Shevchenko wrote:
> > > On Mon, May 11, 2020 at 10:32:55PM +0300, Serge Semin wrote:
> > > > On Mon, May 11, 2020 at 04:58:53PM +0300, Andy Shevchenko wrote:
> > > > > On Mon, May 11, 2020 at 4:48 PM Serge Semin
> > > > > <Sergey.Semin@baikalelectronics.ru> wrote:
> > > > > >
> > > > > > On Mon, May 11, 2020 at 12:58:13PM +0100, Mark Brown wrote:
> > > > > > > On Mon, May 11, 2020 at 05:10:16AM +0300, Serge Semin wrote:
> > > > > > >
> > > > > > > > Alas linearizing the SPI messages won't help in this case because the DW DMA
> > > > > > > > driver will split it into the max transaction chunks anyway.
> > > > > > >
> > > > > > > That sounds like you need to also impose a limit on the maximum message
> > > > > > > size as well then, with that you should be able to handle messages up
> > > > > > > to whatever that limit is. There's code for that bit already, so long
> > > > > > > as the limit is not too low it should be fine for most devices and
> > > > > > > client drivers can see the limit so they can be updated to work with it
> > > > > > > if needed.
> > > > > >
> > > > > > Hmm, this might work. The problem will be with imposing such limitation through
> > > > > > the DW APB SSI driver. In order to do this I need to know:
> > > > > > 1) Whether multi-block LLP is supported by the DW DMA controller.
> > > > > > 2) Maximum DW DMA transfer block size.
> > > > > > Then I'll be able to use this information in the can_dma() callback to enable
> > > > > > the DMA xfers only for the safe transfers. Did you mean something like this when
> > > > > > you said "There's code for that bit already" ? If you meant the max_dma_len
> > > > > > parameter, then setting it won't work, because it just limits the SG items size
> > > > > > not the total length of a single transfer.
> > > > > >
> > > > > > So the question is of how to export the multi-block LLP flag from DW DMAc
> > > > > > driver. Andy?
> > > > >
> > > > > I'm not sure I understand why do you need this being exported. Just
> > > > > always supply SG list out of single entry and define the length
> > > > > according to the maximum segment size (it's done IIRC in SPI core).
> > > >
> > > > Finally I see your point. So you suggest to feed the DMA engine with SG list
> > > > entries one-by-one instead of sending all of them at once in a single
> > > > dmaengine_prep_slave_sg() -> dmaengine_submit() -> dma_async_issue_pending()
> > > > session. Hm, this solution will work, but there is an issue. There is no
> > > > guarantee, that Tx and Rx SG lists are symmetric, consisting of the same
> > > > number of items with the same sizes. It depends on the Tx/Rx buffers physical
> > > > address alignment and their offsets within the memory pages. Though this
> > > > problem can be solved by making the Tx and Rx SG lists symmetric. I'll have
> > > > to implement a clever DMA IO loop, which would extract the DMA
> > > > addresses/lengths from the SG entries and perform the single-buffer DMA
> > > > transactions with the DMA buffers of the same length.
> > > >
> > > > Regarding noLLP being exported. Obviously I intended to solve the problem in a
> > > > generic way since the problem is common for noLLP DW APB SSI/DW DMAC combination.
> > > > In order to do this we need to know whether the multi-block LLP feature is
> > > > unsupported by the DW DMA controller. We either make such info somehow exported
> > > > from the DW DMA driver, so the DMA clients (like Dw APB SSI controller driver)
> > > > could be ready to work around the problem; or just implement a flag-based quirk
> > > > in the DMA client driver, which would be enabled in the platform-specific basis
> > > > depending on the platform device actually detected (for instance, a specific
> > > > version of the DW APB SSI IP). AFAICS You'd prefer the later option.
> > >
> > > So, we may extend the struct of DMA parameters to tell the consumer amount of entries (each of which is no longer than maximum segment size) it can afford:
> > > - 0: Auto (DMA driver handles any cases itself)
> > > - 1: Only single entry
> > > - 2: Up to two...
> >
> > It will left implementation details (or i.o.w. obstacles or limitation) why DMA
> > can't do otherwise.
>
> Sounds good. Thanks for assistance.
>
> -Sergey
>
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >
--
~Vinod
^ permalink raw reply
* Re: [PATCH v7 2/4] usb: dwc3: qcom: Add interconnect support in dwc3 driver
From: Felipe Balbi @ 2020-05-15 6:29 UTC (permalink / raw)
To: Georgi Djakov, Matthias Kaehlcke
Cc: Sandeep Maheswaram, Andy Gross, Bjorn Andersson,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Stephen Boyd,
Doug Anderson, linux-arm-msm, linux-usb, devicetree, linux-kernel,
Manu Gautam, Chandana Kishori Chiluveru, Viresh Kumar
In-Reply-To: <090e48d7-7988-eea1-bf39-f6820578d354@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 2103 bytes --]
Hi,
Georgi Djakov <georgi.djakov@linaro.org> writes:
>>>>>> Sandeep Maheswaram <sanm@codeaurora.org> writes:
>>>>>>> +static int dwc3_qcom_interconnect_init(struct dwc3_qcom *qcom)
>>>>>>> +{
>>>>>>> + struct device *dev = qcom->dev;
>>>>>>> + int ret;
>>>>>>> +
>>>>>>> + if (!device_is_bound(&qcom->dwc3->dev))
>>>>>>> + return -EPROBE_DEFER;
>>>>>>
>>>>>> this breaks allmodconfig. I'm dropping this series from my queue for
>>>>>> this merge window.
>>>>>
>>>>> Sorry, I meant this patch ;-)
>>>>
>>>> I guess that's due to INTERCONNECT being a module. There is currently a
>>>
>>> I believe it's because of this:
>>> ERROR: modpost: "device_is_bound" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!
>>>
>>>> discussion about this with Viresh and Georgi in response to another
>>>> automated build failure. Viresh suggests changing CONFIG_INTERCONNECT
>>>> from tristate to bool, which seems sensible to me given that interconnect
>>>> is a core subsystem.
>>>
>>> The problem you are talking about would arise when INTERCONNECT=m and
>>> USB_DWC3_QCOM=y and it definitely exists here and could be triggered with
>>> randconfig build. So i suggest to squash also the diff below.
>>>
>>> Thanks,
>>> Georgi
>>>
>>> ---8<---
>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>>> index 206caa0ea1c6..6661788b1a76 100644
>>> --- a/drivers/usb/dwc3/Kconfig
>>> +++ b/drivers/usb/dwc3/Kconfig
>>> @@ -129,6 +129,7 @@ config USB_DWC3_QCOM
>>> tristate "Qualcomm Platform"
>>> depends on ARCH_QCOM || COMPILE_TEST
>>> depends on EXTCON || !EXTCON
>>> + depends on INTERCONNECT || !INTERCONNECT
>>
>> I would prefer to see a patch adding EXPORT_SYMBOL_GPL() to device_is_bound()
>
> Agree, but just to clarify, that these are two separate issues that need to
> be fixed. The device_is_bound() is the first one and USB_DWC3_QCOM=y combined
> with INTERCONNECT=m is the second one.
If INTERCONNECT=m, QCOM3 shouldn't be y. I think the following is
enough:
depends on INTERCONNECT=y || INTERCONNECT=USB_DWC3_QCOM
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/6] dmaengine: dw: Set DMA device max segment size parameter
From: Vinod Koul @ 2020-05-15 6:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Serge Semin, Vineet Gupta, Viresh Kumar,
Dan Williams, Alexey Malahov, Thomas Bogendoerfer, Paul Burton,
Ralf Baechle, Arnd Bergmann, Rob Herring, linux-mips, devicetree,
dmaengine, linux-kernel
In-Reply-To: <20200512123551.GX185537@smile.fi.intel.com>
On 12-05-20, 15:35, Andy Shevchenko wrote:
> On Tue, May 12, 2020 at 12:16:22AM +0300, Serge Semin wrote:
> > On Fri, May 08, 2020 at 02:21:52PM +0300, Andy Shevchenko wrote:
> > > On Fri, May 08, 2020 at 01:53:01PM +0300, Serge Semin wrote:
> > > > Maximum block size DW DMAC configuration corresponds to the max segment
> > > > size DMA parameter in the DMA core subsystem notation. Lets set it with a
> > > > value specific to the probed DW DMA controller. It shall help the DMA
> > > > clients to create size-optimized SG-list items for the controller. This in
> > > > turn will cause less dw_desc allocations, less LLP reinitializations,
> > > > better DMA device performance.
>
> > > Yeah, I have locally something like this and I didn't dare to upstream because
> > > there is an issue. We have this information per DMA controller, while we
> > > actually need this on per DMA channel basis.
> > >
> > > Above will work only for synthesized DMA with all channels having same block
> > > size. That's why above conditional is not needed anyway.
> >
> > Hm, I don't really see why the conditional isn't needed and this won't work. As
> > you can see in the loop above Initially I find a maximum of all channels maximum
> > block sizes and use it then as a max segment size parameter for the whole device.
> > If the DW DMA controller has the same max block size of all channels, then it
> > will be found. If the channels've been synthesized with different block sizes,
> > then the optimization will work for the one with greatest block size. The SG
> > list entries of the channels with lesser max block size will be split up
> > by the DW DMAC driver, which would have been done anyway without
> > max_segment_size being set. Here we at least provide the optimization for the
> > channels with greatest max block size.
> >
> > I do understand that it would be good to have this parameter setup on per generic
> > DMA channel descriptor basis. But DMA core and device descriptor doesn't provide
> > such facility, so setting at least some justified value is a good idea.
> >
> > >
> > > OTOH, I never saw the DesignWare DMA to be synthesized differently (I remember
> > > that Intel Medfield has interesting settings, but I don't remember if DMA
> > > channels are different inside the same controller).
> > >
> > > Vineet, do you have any information that Synopsys customers synthesized DMA
> > > controllers with different channel characteristics inside one DMA IP?
> >
> > AFAICS the DW DMAC channels can be synthesized with different max block size.
> > The IP core supports such configuration. So we can't assume that such DMAC
> > release can't be found in a real hardware just because we've never seen one.
> > No matter what Vineet will have to say in response to your question.
>
> My point here that we probably can avoid complications till we have real
> hardware where it's different. As I said I don't remember a such, except
> *maybe* Intel Medfield, which is quite outdated and not supported for wider
> audience anyway.
IIRC Intel Medfield has couple of dma controller instances each one with
different parameters *but* each instance has same channel configuration.
I do not recall seeing that we have synthesis parameters per channel
basis... But I maybe wrong, it's been a while.
--
~Vinod
^ permalink raw reply
* Re: [PATCH v7 2/4] usb: dwc3: qcom: Add interconnect support in dwc3 driver
From: Georgi Djakov @ 2020-05-15 6:11 UTC (permalink / raw)
To: Felipe Balbi, Matthias Kaehlcke
Cc: Sandeep Maheswaram, Andy Gross, Bjorn Andersson,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Stephen Boyd,
Doug Anderson, linux-arm-msm, linux-usb, devicetree, linux-kernel,
Manu Gautam, Chandana Kishori Chiluveru, Viresh Kumar
In-Reply-To: <87tv0h3fpv.fsf@kernel.org>
Hi,
On 5/15/20 08:54, Felipe Balbi wrote:
>
> Hi,
>
> Georgi Djakov <georgi.djakov@linaro.org> writes:
>> On 5/14/20 20:13, Matthias Kaehlcke wrote:
>>> On Thu, May 14, 2020 at 02:30:28PM +0300, Felipe Balbi wrote:
>>>> Felipe Balbi <balbi@kernel.org> writes:
>>>>
>>>>> Hi,
>>>>>
>>>>> Sandeep Maheswaram <sanm@codeaurora.org> writes:
>>>>>> +static int dwc3_qcom_interconnect_init(struct dwc3_qcom *qcom)
>>>>>> +{
>>>>>> + struct device *dev = qcom->dev;
>>>>>> + int ret;
>>>>>> +
>>>>>> + if (!device_is_bound(&qcom->dwc3->dev))
>>>>>> + return -EPROBE_DEFER;
>>>>>
>>>>> this breaks allmodconfig. I'm dropping this series from my queue for
>>>>> this merge window.
>>>>
>>>> Sorry, I meant this patch ;-)
>>>
>>> I guess that's due to INTERCONNECT being a module. There is currently a
>>
>> I believe it's because of this:
>> ERROR: modpost: "device_is_bound" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!
>>
>>> discussion about this with Viresh and Georgi in response to another
>>> automated build failure. Viresh suggests changing CONFIG_INTERCONNECT
>>> from tristate to bool, which seems sensible to me given that interconnect
>>> is a core subsystem.
>>
>> The problem you are talking about would arise when INTERCONNECT=m and
>> USB_DWC3_QCOM=y and it definitely exists here and could be triggered with
>> randconfig build. So i suggest to squash also the diff below.
>>
>> Thanks,
>> Georgi
>>
>> ---8<---
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index 206caa0ea1c6..6661788b1a76 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -129,6 +129,7 @@ config USB_DWC3_QCOM
>> tristate "Qualcomm Platform"
>> depends on ARCH_QCOM || COMPILE_TEST
>> depends on EXTCON || !EXTCON
>> + depends on INTERCONNECT || !INTERCONNECT
>
> I would prefer to see a patch adding EXPORT_SYMBOL_GPL() to device_is_bound()
Agree, but just to clarify, that these are two separate issues that need to
be fixed. The device_is_bound() is the first one and USB_DWC3_QCOM=y combined
with INTERCONNECT=m is the second one.
Thanks,
Georgi
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: dma: dw: Add max burst transaction length property
From: Vinod Koul @ 2020-05-15 6:09 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Serge Semin, Viresh Kumar, Rob Herring,
Alexey Malahov, Thomas Bogendoerfer, Paul Burton, Ralf Baechle,
Arnd Bergmann, Dan Williams, linux-mips, dmaengine, devicetree,
linux-kernel
In-Reply-To: <20200512123840.GY185537@smile.fi.intel.com>
On 12-05-20, 15:38, Andy Shevchenko wrote:
> On Tue, May 12, 2020 at 02:49:46PM +0300, Serge Semin wrote:
> > On Tue, May 12, 2020 at 12:08:04PM +0300, Andy Shevchenko wrote:
> > > On Tue, May 12, 2020 at 12:35:31AM +0300, Serge Semin wrote:
> > > > On Tue, May 12, 2020 at 12:01:38AM +0300, Andy Shevchenko wrote:
> > > > > On Mon, May 11, 2020 at 11:05:28PM +0300, Serge Semin wrote:
> > > > > > On Fri, May 08, 2020 at 02:12:42PM +0300, Andy Shevchenko wrote:
> > > > > > > On Fri, May 08, 2020 at 01:53:00PM +0300, Serge Semin wrote:
>
> ...
>
> I leave it to Rob and Vinod.
> It won't break our case, so, feel free with your approach.
I agree the DT is about describing the hardware and looks like value of
1 is not allowed. If allowed it should be added..
> P.S. Perhaps at some point we need to
> 1) convert properties to be u32 (it will simplify things);
> 2) convert legacy ones to proper format ('-' instead of '_', vendor prefix added);
> 3) parse them in core with device property API.
These suggestions are good and should be done.
--
~Vinod
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox