Devicetree
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: sun8i: add a delay after reset xr819 on Orange Pi Zero
From: Icenowy Zheng @ 2016-12-20  5:58 UTC (permalink / raw)
  To: Maxime Ripard, Chen-Yu Tsai, Hans de Goede
  Cc: devicetree, linux-kernel, linux-arm-kernel, Icenowy Zheng

XR819 seems to need a delay after its reset line to be deasserted,
otherwise it may not respond MMC commands correctly, and fail to
initialize.

Add a 200ms delay in the mmc-pwrseq.

Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
---
 arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
index d18807f73060..b7ca916d871d 100644
--- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
+++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
@@ -92,6 +92,7 @@
 	wifi_pwrseq: wifi_pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		reset-gpios = <&r_pio 0 7 GPIO_ACTIVE_LOW>;
+		post-power-on-delay-ms = <200>;
 	};
 };
 
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v2 4/4] phy: qcom-qmp: new qmp phy driver for qcom-chipsets
From: Vivek Gautam @ 2016-12-20  5:42 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: kishon, robh+dt, Mark Rutland, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Srinivas Kandagatla, linux-arm-msm
In-Reply-To: <20161129003522.GP6095@codeaurora.org>

Hi,

On Tue, Nov 29, 2016 at 6:05 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:

Thanks for a thorough review. Please find my comments inline.

> On 11/22, Vivek Gautam wrote:
>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
>> index f1dcec1..8970d9e 100644
>> --- a/drivers/phy/Kconfig
>> +++ b/drivers/phy/Kconfig
>> @@ -430,6 +430,15 @@ config PHY_STIH407_USB
>>         Enable this support to enable the picoPHY device used by USB2
>>         and USB3 controllers on STMicroelectronics STiH407 SoC families.
>>
>> +config PHY_QCOM_QMP
>> +     tristate "Qualcomm QMP PHY Driver"
>> +     depends on OF && (ARCH_QCOM || COMPILE_TEST)
>> +     select GENERIC_PHY
>> +     select RESET_CONTROLLER
>
> Again, probably should be dropped as we're not providing resets
> here, just using them.

Sure, will drop in v3.

>
>> diff --git a/drivers/phy/phy-qcom-qmp.c b/drivers/phy/phy-qcom-qmp.c
>> new file mode 100644
>> index 0000000..f85289e
>> --- /dev/null
>> +++ b/drivers/phy/phy-qcom-qmp.c
>> @@ -0,0 +1,1141 @@
>> +/*
>> + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/reset.h>
>> +#include <linux/slab.h>
>> +
>> +#include <dt-bindings/phy/phy.h>
>> +
> [...]
>> +
>> +/* set of registers with offsets different per-PHY */
>> +enum qphy_reg_layout {
>> +     /* Common block control registers */
>> +     QPHY_COM_SW_RESET,
>> +     QPHY_COM_POWER_DOWN_CONTROL,
>> +     QPHY_COM_START_CONTROL,
>> +     QPHY_COM_PCS_READY_STATUS,
>> +     /* PCS registers */
>> +     QPHY_PLL_LOCK_CHK_DLY_TIME,
>> +     QPHY_FLL_CNTRL1,
>> +     QPHY_FLL_CNTRL2,
>> +     QPHY_FLL_CNT_VAL_L,
>> +     QPHY_FLL_CNT_VAL_H_TOL,
>> +     QPHY_FLL_MAN_CODE,
>> +     QPHY_PCS_READY_STATUS,
>> +};
>> +
>> +unsigned int pciephy_regs_layout[] = {
>
> static? const?

Will take care of these in v3.
[...]

>> +
>> +/**
>> + * struct qmp_phy_init_cfg:- per-PHY init config.
>
> The colon and dash can't be right. One is kernel-doc, but of
> course there aren't any other member descriptions.

Right, will convert this to a one-line comment.

>
>> + */
>> +struct qmp_phy_init_cfg {
>> +     /* phy-type - PCIE/UFS/USB */
>> +     unsigned int type;
>> +     /* number of lanes provided by phy */
>> +     int nlanes;
>> +
>> +     /* Initialization sequence for PHY blocks - Serdes, tx, rx, pcs */
>> +     struct qmp_phy_init_tbl *phy_init_serdes_tbl;
>> +     int phy_init_serdes_tbl_sz;
>> +     struct qmp_phy_init_tbl *phy_init_tx_tbl;
>> +     int phy_init_tx_tbl_sz;
>> +     struct qmp_phy_init_tbl *phy_init_rx_tbl;
>> +     int phy_init_rx_tbl_sz;
>> +     struct qmp_phy_init_tbl *phy_init_pcs_tbl;
>> +     int phy_init_pcs_tbl_sz;
>
> _sz makes it sound like bytes, perhaps _num instead.

sure.

>
>> +
>> +     /* array of registers with different offsets */
>> +     unsigned int *regs;
>
> const?

taking care of these in v3.

>
>> +
>> +     unsigned int mask_start_ctrl;
>> +     unsigned int mask_pwr_dn_ctrl;
>> +     /* true, if PHY has a separate PHY_COM_CNTRL block */
>> +     bool has_phy_com_ctrl;
>> +     /* true, if PHY has a reset for individual lanes */
>> +     bool has_lane_rst;
>> +};
>> +
>> +/**
>> + * struct qmp_phy_desc:- per-lane phy-descriptor.
>
> Also kernel-doc requests full stop is left out on one line
> descriptions.

sure, will remove it from here and from other places where a full-stop
is not required.

>
>> + *
>> + * @phy: pointer to generic phy
>> + * @tx: pointer to iomapped memory space for PHY's tx
>> + * @rx: pointer to iomapped memory space for PHY's rx
>> + * @pcs: pointer to iomapped memory space for PHY's pcs
>> + * @pipe_clk: pointer to pipe lock
>> + * @index: lane index
>> + * @qphy: pointer to QMP phy to which this lane belongs
>> + * @lane_rst: pointer to lane's reset controller
>
> Not sure we care about "pointer to" as types can be figured out
> other ways.

will drop "pointer to" string.

>
>> + */
>> +struct qmp_phy_desc {
>> +     struct phy *phy;
>> +     void __iomem *tx;
>> +     void __iomem *rx;
>> +     void __iomem *pcs;
>> +     struct clk *pipe_clk;
>> +     unsigned int index;
>> +     struct qcom_qmp_phy *qphy;
>> +     struct reset_control *lane_rst;
>> +};
>> +
>> +/**
>> + * struct qcom_qmp_phy:- structure holding QMP PHY attributes.
>> + *
>> + * @dev: pointer to device
>> + * @serdes: pointer to iomapped memory space for phy's serdes
>> + *
>> + * @aux_clk: pointer to phy core clock
>> + * @cfg_ahb_clk: pointer to AHB2PHY interface clock
>> + * @ref_clk: pointer to reference clock
>> + * @ref_clk_src: pointer to source to reference clock
>> + *
>> + * @vdda_phy: vdd supply to the phy core block
>> + * @vdda_pll: 1.8V vdd supply to ref_clk block
>> + * @vddp_ref_clk: vdd supply to specific ref_clk block (Optional)
>> + *
>> + * @phy_rst: Pointer to phy reset control
>> + * @phycom_rst: Pointer to phy common reset control
>> + * @phycfg_rst: Pointer to phy ahb cfg reset control (Optional)
>> + *
>> + * @cfg: pointer to init config for each phys
>> + * @phys: array of pointer to per-lane phy descriptors
>> + * @phy_mutex: mutex lock for PHY common block initialization
>> + * @init_count: Phy common block initialization count
>> + */
>> +struct qcom_qmp_phy {
>> +     struct device *dev;
>> +     void __iomem *serdes;
>> +
>> +     struct clk *aux_clk;
>> +     struct clk *cfg_ahb_clk;
>> +     struct clk *ref_clk;
>> +     struct clk *ref_clk_src;
>> +
>> +     struct regulator *vdda_phy;
>> +     struct regulator *vdda_pll;
>> +     struct regulator *vddp_ref_clk;
>> +
>> +     struct reset_control *phy_rst;
>> +     struct reset_control *phycom_rst;
>> +     struct reset_control *phycfg_rst;
>> +
>> +     const struct qmp_phy_init_cfg *cfg;
>> +     struct qmp_phy_desc **phys;
>> +
>> +     struct mutex phy_mutex;
>> +     int init_count;
>> +};
>> +
>> +static inline void qphy_setbits(void __iomem *reg, u32 val)
>> +{
>> +     u32 reg_val;
>> +
>> +     reg_val = readl_relaxed(reg);
>> +     reg_val |= val;
>> +     writel_relaxed(reg_val, reg);
>> +}
>> +
>> +static inline void qphy_clrbits(void __iomem *reg, u32 val)
>> +{
>> +     u32 reg_val;
>> +
>> +     reg_val = readl_relaxed(reg);
>> +     reg_val &= ~val;
>> +     writel_relaxed(reg_val, reg);
>> +}
>> +
>> +const struct qmp_phy_init_cfg msm8996_pciephy_init_cfg = {
>
> static?
>
>> +     .type                   = PHY_TYPE_PCIE,
>> +     .nlanes                 = 3,
>> +
>> +     .phy_init_serdes_tbl    = pciephy_serdes_init_tbl,
>> +     .phy_init_serdes_tbl_sz = ARRAY_SIZE(pciephy_serdes_init_tbl),
>> +     .phy_init_tx_tbl        = pciephy_tx_init_tbl,
>> +     .phy_init_tx_tbl_sz     = ARRAY_SIZE(pciephy_tx_init_tbl),
>> +     .phy_init_rx_tbl        = pciephy_rx_init_tbl,
>> +     .phy_init_rx_tbl_sz     = ARRAY_SIZE(pciephy_rx_init_tbl),
>> +     .phy_init_pcs_tbl       = pciephy_pcs_init_tbl,
>> +     .phy_init_pcs_tbl_sz    = ARRAY_SIZE(pciephy_pcs_init_tbl),
>> +     .regs                   = pciephy_regs_layout,
>> +     .mask_start_ctrl        = (PHY_PCS_START | PHY_PLL_READY_GATE_EN),
>> +     .mask_pwr_dn_ctrl       = (PHY_SW_PWRDN | PHY_REFCLK_DRV_DSBL),
>> +
>> +     .has_phy_com_ctrl       = true,
>> +     .has_lane_rst           = true,
>> +};
>> +
>> +const struct qmp_phy_init_cfg msm8996_usb3phy_init_cfg = {
>
> static? Try running sparse.

Right, will do that from now on.

>
>> +     .type                   = PHY_TYPE_USB3,
>> +     .nlanes                 = 1,
>> +
>> +     .phy_init_serdes_tbl    = usb3phy_serdes_init_tbl,
>> +     .phy_init_serdes_tbl_sz = ARRAY_SIZE(usb3phy_serdes_init_tbl),
>> +     .phy_init_tx_tbl        = usb3phy_tx_init_tbl,
>> +     .phy_init_tx_tbl_sz     = ARRAY_SIZE(usb3phy_tx_init_tbl),
>> +     .phy_init_rx_tbl        = usb3phy_rx_init_tbl,
>> +     .phy_init_rx_tbl_sz     = ARRAY_SIZE(usb3phy_rx_init_tbl),
>> +     .phy_init_pcs_tbl       = usb3phy_pcs_init_tbl,
>> +     .phy_init_pcs_tbl_sz    = ARRAY_SIZE(usb3phy_pcs_init_tbl),
>
> Do we really need phy_init as a prefix. Could just be serdes_tbl,
> tx_tbl, etc?

Right, shorter names improve readability.

>
>> +     .regs                   = usb3phy_regs_layout,
>> +     .mask_start_ctrl        = (PHY_SERDES_START | PHY_PCS_START),
>> +     .mask_pwr_dn_ctrl       = PHY_SW_PWRDN,
>> +};
>> +
>> +static void qcom_qmp_phy_configure(void __iomem *base,
>> +                             unsigned int *regs_layout,
>> +                             struct qmp_phy_init_tbl init_tbl[],
>> +                             int init_tbl_sz)
>
> Shorter variable names would make this easier to read.

ditto.

>
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < init_tbl_sz; i++) {
>> +             if (init_tbl[i].in_layout)
>> +                     writel_relaxed(init_tbl[i].cfg_val,
>> +                             base + regs_layout[init_tbl[i].reg_offset]);
>> +             else
>> +                     writel_relaxed(init_tbl[i].cfg_val,
>> +                             base + init_tbl[i].reg_offset);
>> +     }
>> +
>         const struct qmp_phy_init_tbl *t = tbl;
>
>         for (i = 0; i < num; i++, t++)
>                 if (t->in_layout)
>                         writel_relaxed(t->val, base + regs[t->offset]);
>                 else
>                         writel_relaxed(t->val, base + t->offset);
>

Updating this in v3.

>> +     /* flush buffered writes */
>> +     mb();
>> +}
>> +
>> +static int qcom_qmp_phy_poweron(struct phy *phy)
>> +{
>> +     struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
>> +     struct qcom_qmp_phy *qphy = phydesc->qphy;
>> +     int ret;
>> +
>> +     dev_vdbg(&phy->dev, "Powering on QMP phy\n");
>> +
>> +     ret = regulator_enable(qphy->vdda_phy);
>> +     if (ret) {
>> +             dev_err(qphy->dev, "%s: vdda-phy enable failed, err=%d\n",
>> +                             __func__, ret);
>> +             return ret;
>> +     }
>> +
>> +     ret = regulator_enable(qphy->vdda_pll);
>> +     if (ret) {
>> +             dev_err(qphy->dev, "%s: vdda-pll enable failed, err=%d\n",
>> +                             __func__, ret);
>> +             goto err_vdda_pll;
>> +     }
>> +
>> +     if (qphy->vddp_ref_clk) {
>> +             ret = regulator_enable(qphy->vddp_ref_clk);
>> +             if (ret) {
>> +                     dev_err(qphy->dev, "%s: vdda-ref-clk enable failed, err=%d\n",
>> +                                     __func__, ret);
>> +                     goto err_vddp_refclk;
>> +             }
>> +     }
>> +
>> +     clk_prepare_enable(qphy->ref_clk_src);
>> +     clk_prepare_enable(qphy->ref_clk);
>> +     clk_prepare_enable(phydesc->pipe_clk);
>
> And if these fail?

Will add error handling for all instances of clk_prepare_enable()
across the driver in next patch version.

>
>> +
>> +     return 0;
>> +
>> +err_vddp_refclk:
>> +     regulator_disable(qphy->vdda_pll);
>> +err_vdda_pll:
>> +     regulator_disable(qphy->vdda_phy);
>> +     return ret;
>> +}
>> +
>> +static int qcom_qmp_phy_poweroff(struct phy *phy)
>> +{
>> +     struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
>> +     struct qcom_qmp_phy *qphy = phydesc->qphy;
>> +
>> +     clk_disable_unprepare(qphy->ref_clk_src);
>> +     clk_disable_unprepare(qphy->ref_clk);
>> +     clk_disable_unprepare(phydesc->pipe_clk);
>> +
>> +     if (qphy->vddp_ref_clk)
>> +             regulator_disable(qphy->vddp_ref_clk);
>> +
>> +     regulator_disable(qphy->vdda_pll);
>> +     regulator_disable(qphy->vdda_phy);
>> +
>> +     return 0;
>> +}
>> +
>> +static int qcom_qmp_phy_is_ready(struct qcom_qmp_phy *qphy,
>> +                             void __iomem *pcs_status, u32 mask)
>> +{
>> +     unsigned int init_timeout;
>> +
>> +     init_timeout = PHY_READY_TIMEOUT_COUNT;
>> +     do {
>> +             if (readl_relaxed(pcs_status) & mask)
>> +                     break;
>> +
>> +             usleep_range(REFCLK_STABILIZATION_DELAY_US_MIN,
>> +                              REFCLK_STABILIZATION_DELAY_US_MAX);
>> +     } while (--init_timeout);
>
> readl_poll_timeout?

Right, will use that.

>
>> +
>> +     if (!init_timeout)
>> +             return -EBUSY;
>> +
>> +     return 0;
>> +}
>> +
> [...]
>> +
>> +/* PHY Initialization */
>> +static int qcom_qmp_phy_init(struct phy *phy)
>> +{
>> +     struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
>> +     struct qcom_qmp_phy *qphy = phydesc->qphy;
>> +     const struct qmp_phy_init_cfg *cfg = qphy->cfg;
>> +     void __iomem *tx = phydesc->tx;
>> +     void __iomem *rx = phydesc->rx;
>> +     void __iomem *pcs = phydesc->pcs;
>> +     int ret;
>> +
>> +     dev_vdbg(qphy->dev, "Initializing QMP phy\n");
>> +
>> +     /* enable interface clocks to program phy */
>> +     clk_prepare_enable(qphy->aux_clk);
>> +     clk_prepare_enable(qphy->cfg_ahb_clk);
>> +
>> +     ret = qcom_qmp_phy_com_init(qphy);
>> +     if (ret)
>> +             goto err;
>> +
>> +     if (phydesc->lane_rst) {
>> +             ret = reset_control_deassert(phydesc->lane_rst);
>> +             if (ret) {
>> +                     dev_err(qphy->dev, "lane<%d> reset deassert failed\n",
>
> Drop the brackets?
sure.

>
>> +                                     phydesc->index);
>> +                     goto err_lane_rst;
>> +             }
>> +     }
>> +
>> +     /* Tx, Rx, and PCS configurations */
>> +     qcom_qmp_phy_configure(tx, cfg->regs, cfg->phy_init_tx_tbl,
>> +                             cfg->phy_init_tx_tbl_sz);
>> +     qcom_qmp_phy_configure(rx, cfg->regs, cfg->phy_init_rx_tbl,
>> +                             cfg->phy_init_rx_tbl_sz);
>> +     qcom_qmp_phy_configure(pcs, cfg->regs, cfg->phy_init_pcs_tbl,
>> +                             cfg->phy_init_pcs_tbl_sz);
>> +
>> +     /*
>> +      * Pull out PHY from POWER DOWN state:
>> +      * This is active low enable signal to power-down PHY.
>> +      */
>> +     qphy_setbits(pcs + QPHY_POWER_DOWN_CONTROL, cfg->mask_pwr_dn_ctrl);
>> +     /* XXX: 10 us delay; given in PCIE phy programming guide only */
>
> Is this a todo?

No, this delay seems to be required by the pcie phy only. USB3 phy guide
doesn't mention anything about this delay. Neither the downstream
USB3 qmp phy driver uses this delay.
Can add a check for the phy type to use this delay.

>
>> +     usleep_range(POWER_DOWN_DELAY_US_MIN, POWER_DOWN_DELAY_US_MAX);
>> +
>> +     /* start SerDes and Phy-Coding-Sublayer */
>> +     qphy_setbits(pcs + QPHY_START_CTRL, cfg->mask_start_ctrl);
>> +
>> +     /* Pull PHY out of reset state */
>> +     qphy_clrbits(pcs + QPHY_SW_RESET, PHY_SW_RESET);
>> +     /* Make sure that above writes are completed */
>> +     mb();
>> +
>> +     ret = qcom_qmp_phy_is_ready(qphy, pcs +
>> +                                     cfg->regs[QPHY_PCS_READY_STATUS],
>> +                                     MASK_PHYSTATUS);
>> +     if (ret) {
>> +             dev_err(qphy->dev, "phy initialization timed-out\n");
>> +             goto err_pcs_ready;
>> +     }
>> +
>> +     return 0;
>> +
>> +err_pcs_ready:
>> +     if (phydesc->lane_rst)
>> +             reset_control_assert(phydesc->lane_rst);
>> +err_lane_rst:
>> +     qcom_qmp_phy_com_exit(qphy);
>> +err:
>> +     clk_disable_unprepare(qphy->cfg_ahb_clk);
>> +     clk_disable_unprepare(qphy->aux_clk);
>> +     return ret;
>> +}
>> +
>> +static int qcom_qmp_phy_exit(struct phy *phy)
>> +{
>> +     struct qmp_phy_desc *phydesc = phy_get_drvdata(phy);
>> +     struct qcom_qmp_phy *qphy = phydesc->qphy;
>> +     const struct qmp_phy_init_cfg *cfg = qphy->cfg;
>> +
>> +     /* PHY reset */
>> +     qphy_setbits(phydesc->pcs + QPHY_SW_RESET, PHY_SW_RESET);
>> +
>> +     /* stop SerDes and Phy-Coding-Sublayer */
>> +     qphy_clrbits(phydesc->pcs + QPHY_START_CTRL, cfg->mask_start_ctrl);
>> +
>> +     /* Put PHY into POWER DOWN state: active low */
>> +     qphy_clrbits(phydesc->pcs + QPHY_POWER_DOWN_CONTROL,
>> +                     cfg->mask_pwr_dn_ctrl);
>> +
>> +     /* Make sure that above writes are completed */
>> +     mb();
>> +
>> +     if (phydesc->lane_rst)
>> +             reset_control_assert(phydesc->lane_rst);
>> +
>> +     qcom_qmp_phy_com_exit(qphy);
>> +
>> +     clk_disable_unprepare(qphy->aux_clk);
>> +     clk_disable_unprepare(qphy->cfg_ahb_clk);
>> +
>> +     return 0;
>> +}
>> +
>> +
>> +static int qcom_qmp_phy_regulator_init(struct device *dev)
>> +{
>> +     struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
>> +     int ret;
>> +
>> +     qphy->vdda_phy = devm_regulator_get(dev, "vdda-phy");
>> +     if (IS_ERR(qphy->vdda_phy)) {
>> +             ret = PTR_ERR(qphy->vdda_phy);
>> +             if (ret != -EPROBE_DEFER)
>> +                     dev_err(dev, "failed to get vdda-phy, %d\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     qphy->vdda_pll = devm_regulator_get(dev, "vdda-pll");
>> +     if (IS_ERR(qphy->vdda_pll)) {
>> +             ret = PTR_ERR(qphy->vdda_pll);
>> +             if (ret != -EPROBE_DEFER)
>> +                     dev_err(dev, "failed to get vdda-pll, %d\n", ret);
>> +             return ret;
>> +     }
>> +
>> +     /* optional regulator */
>> +     qphy->vddp_ref_clk = devm_regulator_get(dev, "vddp-ref-clk");
>> +     if (IS_ERR(qphy->vddp_ref_clk)) {
>> +             ret = PTR_ERR(qphy->vddp_ref_clk);
>> +             if (ret != -EPROBE_DEFER) {
>> +                     dev_dbg(dev, "failed to get vddp-ref-clk, %d\n", ret);
>> +                     qphy->vddp_ref_clk = NULL;
>> +             } else {
>> +                     return ret;
>> +             }
>> +     }
>
> Regulator framework should insert a dummy regulator if this is
> missing in DT. So we shouldn't need to do anything complicated
> here right?

Right, we don't need to set regulator to NULL. Will simplify this
error handling.

>
>> +
>> +     return 0;
>> +}
>> +
>> +static int qcom_qmp_phy_clk_init(struct device *dev)
>> +{
>> +     struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
>> +     int ret;
>> +
>> +     qphy->aux_clk = devm_clk_get(dev, "aux");
>> +     if (IS_ERR(qphy->aux_clk)) {
>> +             ret = PTR_ERR(qphy->aux_clk);
>> +             if (ret != -EPROBE_DEFER)
>> +                     dev_err(dev, "failed to get aux_clk\n");
>> +             return ret;
>> +     }
>> +
>> +     qphy->cfg_ahb_clk = devm_clk_get(dev, "cfg_ahb");
>> +     if (IS_ERR(qphy->cfg_ahb_clk)) {
>> +             ret = PTR_ERR(qphy->cfg_ahb_clk);
>> +             if (ret != -EPROBE_DEFER)
>> +                     dev_err(dev, "failed to get cfg_ahb_clk\n");
>> +             return ret;
>> +     }
>> +
>> +     qphy->ref_clk_src = devm_clk_get(dev, "ref_clk_src");
>> +     if (IS_ERR(qphy->ref_clk_src)) {
>> +             ret = PTR_ERR(qphy->ref_clk_src);
>> +             if (ret != -EPROBE_DEFER)
>> +                     dev_err(dev, "failed to get ref_clk_src\n");
>> +             return ret;
>> +     }
>> +
>> +     qphy->ref_clk = devm_clk_get(dev, "ref_clk");
>> +     if (IS_ERR(qphy->ref_clk)) {
>> +             ret = PTR_ERR(qphy->ref_clk);
>> +             if (ret != -EPROBE_DEFER)
>> +                     dev_err(dev, "failed to get ref_clk\n");
>> +             return ret;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static struct phy *qcom_qmp_phy_xlate(struct device *dev,
>> +                                     struct of_phandle_args *args)
>> +{
>> +     struct qcom_qmp_phy *qphy = dev_get_drvdata(dev);
>> +     int i;
>> +
>> +     if (WARN_ON(args->args[0] >= qphy->cfg->nlanes))
>> +             return ERR_PTR(-ENODEV);
>> +
>> +     for (i = 0; i < qphy->cfg->nlanes; i++) {
>> +             if (qphy->phys[i]->index == args->args[0])
>> +                     break;
>> +     }
>
> Drop the braces please.

sure.

>
>> +
>> +     if (i == qphy->cfg->nlanes)
>> +             return ERR_PTR(-ENODEV);
>> +
>> +     return qphy->phys[i]->phy;
>
> Could be simplified:
>
>
>         for (i = 0; i < qphy->cfg->nlanes; i++)
>                 if (qphy->phys[i]->index == args->args[0])
>                         return qphy->phys[i]->phy;
>
>         return ERR_PTR(-ENODEV);
>
>
> Also, isn't i == phys[i]->index so this could be a direct lookup?

Thanks. Will modify this as suggested, and will use
if (i == args->args[0]) as the condition.

>
>> +}
>> +
>> +static const struct phy_ops qcom_qmp_phy_gen_ops = {
>> +     .init           = qcom_qmp_phy_init,
>> +     .exit           = qcom_qmp_phy_exit,
>> +     .power_on       = qcom_qmp_phy_poweron,
>> +     .power_off      = qcom_qmp_phy_poweroff,
>> +     .owner          = THIS_MODULE,
>> +};
>> +
>> +static const struct of_device_id qcom_qmp_phy_of_match_table[] = {
>> +     {
>> +             .compatible = "qcom,msm8996-qmp-pcie-phy",
>> +             .data = &msm8996_pciephy_init_cfg,
>> +     }, {
>> +             .compatible = "qcom,msm8996-qmp-usb3-phy",
>> +             .data = &msm8996_usb3phy_init_cfg,
>> +     },
>> +     { },
>> +};
>> +MODULE_DEVICE_TABLE(of, qcom_qmp_phy_of_match_table);
>> +
>> +static int qcom_qmp_phy_probe(struct platform_device *pdev)
>> +{
>> +     struct qcom_qmp_phy *qphy;
>> +     struct device *dev = &pdev->dev;
>> +     struct phy_provider *phy_provider;
>> +     struct resource *res;
>> +     const struct of_device_id *match;
>> +     void __iomem *base;
>> +     int ret = 0;
>
> Shouldn't need a default assignment here.
will remove the assignment.

>
>> +     int id;
>> +
>> +     qphy = devm_kzalloc(dev, sizeof(*qphy), GFP_KERNEL);
>> +     if (!qphy)
>> +             return -ENOMEM;
>> +
>> +     qphy->dev = dev;
>> +     dev_set_drvdata(dev, qphy);
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     base = devm_ioremap_resource(dev, res);
>> +     if (IS_ERR(base))
>> +             return PTR_ERR(base);
>> +
>> +     /* per PHY serdes; usually located at base address */
>> +     qphy->serdes = base;
>> +
>> +     mutex_init(&qphy->phy_mutex);
>> +
>> +     /* Get the specific init parameters of QMP phy */
>> +     match = of_match_node(qcom_qmp_phy_of_match_table, dev->of_node);
>> +     qphy->cfg = match->data;
>> +
>> +     ret = qcom_qmp_phy_clk_init(dev);
>> +     if (ret) {
>> +             dev_err(dev, "clock init failed\n");
>> +             return ret;
>> +     }
>> +
>> +     ret = qcom_qmp_phy_regulator_init(dev);
>> +     if (ret) {
>> +             dev_err(dev, "regulator init failed\n");
>> +             return ret;
>> +     }
>> +
>> +     qphy->phy_rst = devm_reset_control_get(dev, "phy");
>> +     if (IS_ERR(qphy->phy_rst)) {
>> +             dev_err(dev, "failed to get phy core reset\n");
>> +             return PTR_ERR(qphy->phy_rst);
>> +     }
>> +
>> +     qphy->phycom_rst = devm_reset_control_get(dev, "common");
>> +     if (IS_ERR(qphy->phycom_rst)) {
>> +             dev_err(dev, "failed to get phy common reset\n");
>> +             return PTR_ERR(qphy->phycom_rst);
>> +     }
>> +
>> +     qphy->phycfg_rst = devm_reset_control_get(dev, "cfg");
>> +     if (IS_ERR(qphy->phycfg_rst)) {
>> +             dev_dbg(dev, "failed to get phy ahb cfg reset\n");
>> +             qphy->phycfg_rst = NULL;
>> +     }
>> +
>> +     qphy->phys = devm_kcalloc(dev, qphy->cfg->nlanes,
>> +                                     sizeof(*qphy->phys), GFP_KERNEL);
>> +     if (!qphy->phys)
>> +             return -ENOMEM;
>> +
>> +     for (id = 0; id < qphy->cfg->nlanes; id++) {
>> +             struct phy *generic_phy;
>> +             struct qmp_phy_desc *phy_desc;
>> +             char prop_name[MAX_PROP_NAME];
>> +             unsigned int lane_offsets[3];
>> +
>> +             /* mem resources from index 1 to N for N number of lanes */
>> +             res = platform_get_resource(pdev, IORESOURCE_MEM, id + 1);
>> +             base = devm_ioremap_resource(dev, res);
>> +             if (IS_ERR(base))
>> +                     return PTR_ERR(base);
>> +
>> +             phy_desc = devm_kzalloc(dev, sizeof(*phy_desc), GFP_KERNEL);
>> +             if (!phy_desc)
>> +                     return -ENOMEM;
>> +
>> +             /*
>> +              * read offsets to Tx, Rx, and PCS blocks into a u32 array:
>> +              *  ------------------------------------
>> +              * | tx offset | rx offset | pcs offset |
>> +              *  ------------------------------------
>> +              */
>> +             ret = of_property_read_u32_array(dev->of_node, "lane-offsets",
>> +                                                        lane_offsets, 3);
>> +             if (ret) {
>> +                     dev_err(dev,
>> +                             "failed to get tx/rx/pcs offsets for lane%d\n",
>> +                             id);
>> +                             return ret;
>> +             }
>> +
>> +             phy_desc->tx = base + lane_offsets[0];
>> +             phy_desc->rx = base + lane_offsets[1];
>> +             phy_desc->pcs = base + lane_offsets[2];
>> +
>> +             /*
>> +              * Get PHY's Pipe clock, if any; USB3 and PCIe are PIPE3
>> +              * based phys, so they essentially have pipe clock. So,
>> +              * we return error in case phy is USB3 or PIPE type.
>> +              * Otherwise, we initialize pipe clock to NULL for
>> +              * all phys that don't need this.
>> +              */
>> +             memset(&prop_name, 0, sizeof(prop_name));
>> +             snprintf(prop_name, MAX_PROP_NAME, "pipe%d", id);
>> +             phy_desc->pipe_clk = devm_clk_get(dev, prop_name);
>> +             if (IS_ERR(phy_desc->pipe_clk)) {
>> +                     if (qphy->cfg->type == PHY_TYPE_PCIE ||
>> +                         qphy->cfg->type == PHY_TYPE_USB3) {
>> +                             ret = PTR_ERR(phy_desc->pipe_clk);
>> +                             if (ret != -EPROBE_DEFER)
>> +                                     dev_err(dev,
>> +                                     "failed to get lane%d pipe_clk\n", id);
>> +                             return ret;
>> +                     } else {
>> +                             phy_desc->pipe_clk = NULL;
>> +                     }
>
> Drop the else part.

sure.

>
>> +             }
>> +
>> +             /* Get lane reset, if any */
>> +             if (qphy->cfg->has_lane_rst) {
>> +                     memset(&prop_name, 0, sizeof(prop_name));
>> +                     snprintf(prop_name, MAX_PROP_NAME, "lane%d", id);
>> +                     phy_desc->lane_rst = devm_reset_control_get(dev,
>> +                                                                 prop_name);
>> +                     if (IS_ERR(phy_desc->lane_rst)) {
>> +                             dev_err(dev, "failed to get lane%d reset\n",
>> +                                     id);
>> +                             return PTR_ERR(phy_desc->lane_rst);
>> +                     }
>> +             }
>> +
>> +             generic_phy = devm_phy_create(dev, NULL, &qcom_qmp_phy_gen_ops);
>> +             if (IS_ERR(generic_phy)) {
>> +                     ret = PTR_ERR(generic_phy);
>> +                     dev_err(dev, "failed to create qphy %d\n", ret);
>> +                     return ret;
>> +             }
>> +
>> +             phy_desc->phy = generic_phy;
>> +             phy_desc->index = id;
>> +             phy_desc->qphy = qphy;
>> +             phy_set_drvdata(generic_phy, phy_desc);
>> +             qphy->phys[id] = phy_desc;
>
> Probably should make the above part of this loop a function that
> returns a phy for each lane (or an  error code on failure). This
> probe function is long.

Sure, will take this phy creation part to a separate function.


Best Regards
Vivek

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

^ permalink raw reply

* Re: [PATCH v10 0/8] power: add power sequence library
From: Peter Chen @ 2016-12-20  4:31 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Peter Chen, mark.rutland, ulf.hansson, heiko, stephen.boyd,
	linux-kernel, gary.bisson, festevam, stillcompiling, arnd,
	dbaryshkov, vaibhav.hiremath, mka, stern, devicetree, mail,
	pawel.moll, linux-pm, s.hauer, troy.kisky, robh+dt,
	linux-arm-kernel, oscar, gregkh, linux-usb, rjw, sre, broonie,
	p.zabel, shawnguo
In-Reply-To: <20161219191504.GA18988@kozik-lap>

On Mon, Dec 19, 2016 at 09:15:04PM +0200, Krzysztof Kozlowski wrote:
> On Mon, Nov 14, 2016 at 09:35:51AM +0800, Peter Chen wrote:
> > Hi all,
> > 
> > This is a follow-up for my last power sequence framework patch set [1].
> > According to Rob Herring and Ulf Hansson's comments[2]. The kinds of
> > power sequence instances will be added at postcore_initcall, the match
> > criteria is compatible string first, if the compatible string is not
> > matched between dts and library, it will try to use generic power sequence.
> > 	 
> > The host driver just needs to call of_pwrseq_on/of_pwrseq_off
> > if only one power sequence instance is needed, for more power sequences
> > are used, using of_pwrseq_on_list/of_pwrseq_off_list instead (eg, USB hub driver).
> > 
> > In future, if there are special power sequence requirements, the special
> > power sequence library can be created.
> > 
> > This patch set is tested on i.mx6 sabresx evk using a dts change, I use
> > two hot-plug devices to simulate this use case, the related binding
> > change is updated at patch [1/6], The udoo board changes were tested
> > using my last power sequence patch set.[3]
> > 
> > Except for hard-wired MMC and USB devices, I find the USB ULPI PHY also
> > need to power on itself before it can be found by ULPI bus.
> > 
> > [1] http://www.spinics.net/lists/linux-usb/msg142755.html
> > [2] http://www.spinics.net/lists/linux-usb/msg143106.html
> > [3] http://www.spinics.net/lists/linux-usb/msg142815.html
> > 
> > Changes for v10:
> > - Improve the kernel-doc for power sequence core, including exported APIs and
> >   main structure. [Patch 2/8]
> > - Change Kconfig, and let the user choose power sequence. [Patch 2/8]
> > - Delete EXPORT_SYMBOL and change related APIs as local, these APIs do not
> >   be intended to export currently. [Patch 2/8]
> > - Selete POWER_SEQUENCE at USB core's Kconfig. [Patch 4/8]
> 
> Hi Peter,
> 
> It is great that you continued the work on this.
> 
> I saw (in some previous mails) your repo mentioned:
> https://git.kernel.org/cgit/linux/kernel/git/peter.chen/usb.git/
> Does it contain the recent version of this patchset?
> 
> I want to build on top of it fixes for usb3503 on Odroid U3 board.

Krzysztof, I put v10 patch set at branch: pwrseq-lib. 
It seems there are no more comments I will send my v11 patch set
after new year holiday.

-- 

Best Regards,
Peter Chen

^ permalink raw reply

* Re: [PATCH linux v1 0/4] Seven segment display support
From: Jaghathiswari Rankappagounder Natarajan @ 2016-12-20  4:06 UTC (permalink / raw)
  To: David Daney
  Cc: Arnd Bergmann, Neil Armstrong, Thomas Petazzoni, mark.rutland,
	devicetree, Greg KH, OpenBMC Maillist, linux, linux-kernel,
	robh+dt, Joel Stanley, linux-arm-kernel
In-Reply-To: <d054a18a-10d1-7aa5-31d5-44af1e5a6c76@gmail.com>

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

Hi,
Thanks all for the feedback; Further reimplementation of this driver will
be handled by
another member of my team. Please contact Nancy Yuen(yuenn@google.com) for
any details.
Summary of main points mentioned in the feedback were :
  - Address Russell's errno comments
  - Rebase per Joel's comment
  - Research fingerprint reader support (mentioned by Greg KH)
  - Research drivers/leds (mentioned by Arnd)
  - Reimplement either by making an API that all devices of this type could
use or in drivers/leds             model or the fingerprint reader model.



On Wed, Dec 14, 2016 at 12:05 PM, David Daney <ddaney.cavm@gmail.com> wrote:

> On 12/14/2016 06:15 AM, Arnd Bergmann wrote:
>
>> On Wednesday, December 14, 2016 2:12:41 PM CET Neil Armstrong wrote:
>>
>>> On 12/14/2016 01:56 PM, Greg KH wrote:
>>>
>>>> On Wed, Dec 14, 2016 at 01:45:30PM +0100, Thomas Petazzoni wrote:
>>>>
>>>>> Hello,
>>>>>
>>>>> On Tue, 13 Dec 2016 23:55:00 -0800, Jaghathiswari Rankappagounder
>>>>> Natarajan wrote:
>>>>>
>>>>> Documentation for the binding which provides an interface for adding
>>>>>> clock,
>>>>>> data and clear signal GPIO lines to control seven segment display.
>>>>>>
>>>>>> The platform device driver provides an API for displaying on two
>>>>>> 7-segment
>>>>>> displays, and implements the required bit-banging. The hardware
>>>>>> assumed is
>>>>>> 74HC164 wired to two 7-segment displays.
>>>>>>
>>>>>> The character device driver implements the user-space API for letting
>>>>>> a user
>>>>>> write to two 7-segment displays including any conversion methods
>>>>>> necessary
>>>>>> to map the user input to two 7-segment displays.
>>>>>>
>>>>>> Adding clock, data and clear signal GPIO lines in the devicetree to
>>>>>> control
>>>>>> seven segment display on zaius platform.
>>>>>>
>>>>>> The platform driver matches on the device tree node; the platform
>>>>>> driver also
>>>>>> initializes the character device.
>>>>>>
>>>>>> Tested that the seven segment display works properly by writing to the
>>>>>> character device file on a EVB AST2500 board which also has 74HC164
>>>>>> wired
>>>>>> to two 7-segment displays.
>>>>>>
>>>>>
>>>>> FWIW, I proposed a driver for seven segment displays back in 2013:
>>>>>
>>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-
>>>>> January/139986.html
>>>>>
>>>>> And the feedback from Greg KH was: we don't need a driver for that, do
>>>>> it from userspace. See:
>>>>>
>>>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2013-
>>>>> January/139992.html
>>>>>
>>>>> So: good luck
>>>>>
>>>>
>>>> Did anyone ever write a library for this type of thing?
>>>>
>>>> Again, I don't want to see one-off drivers for random devices like this
>>>> that should be able to all be controlled from userspace in a common
>>>> manner.  Much like we did for fingerprint readers a long long time
>>>> ago...
>>>>
>>>>
>> Actually, it's more than a random interface, a lot of SoCs and boards
>>> actually have such displays
>>> and it's a pity to use UIO, sysfs gpio bitbanging and all sort of ugly
>>> stuff to only print a few
>>> characters a simple and clean driver could achieve.
>>> Some very well known SoCs even have integrated registers to lower the
>>> BOM and bypass the need for
>>> a 74HC164 like component and avoid gpio bit banging.
>>>
>>> My personal concern is that you could also need to drive more segments,
>>> thus 7-segments
>>> is too restrictive.
>>>
>>> But this driver is well structured, the gpio-bitbanging sub-driver is
>>> welcome.
>>>
>>
>> Maybe we can find a way to fit this into the existing drivers/leds/
>> subsystem?
>>
>> That already supports blinking, brightness and colour attributes of LEDs,
>> so could this be extended to support (one of) digit, number, character
>> or string with a common sysfs attribute and a way to hook up a led driver
>> to that?
>>
>
> We have a lot of boards with an 8-cell dot matrix LED. Each cell is
> programmed with an 8-bit value.  The mapping of these values to the dots
> defaults to ASCII character rendering, but there is the facility to install
> other bitmaps as well.
>
> Really I view these things not as part of the LED subsystem, but more as a
> very small frame buffer.
>
> We like to display entire words, and the most useful interface from a user
> point of view is something that consumes entire strings rather than having
> to manage each cell independently.
>
> You could imagine that if the text to be displayed were longer than the
> display, that the driver would make it continuously scroll.  I would like
> to see a framework where a simple character device were exposed, and from
> userspace you could do: "echo message > /dev/small-display" and get
> something sensible.
>
>
>>         Arnd
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 7130 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/3] Bluetooth: btusb: Add out-of-band wakeup support
From: Brian Norris @ 2016-12-20  3:41 UTC (permalink / raw)
  To: Rajat Jain
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Amitkumar Karwar, Wei-Ning Huang, Xinming Hu,
	netdev, devicetree, linux-bluetooth, linux-kernel, Rajat Jain
In-Reply-To: <CACK8Z6FteRYmrNq5Oq9LpHOUOYg49XrL0WsxD8G45pTw_cwaSg@mail.gmail.com>

On Mon, Dec 19, 2016 at 05:46:19PM -0800, Rajat Jain wrote:
> On Mon, Dec 19, 2016 at 3:10 PM, Brian Norris <briannorris@chromium.org> wrote:
> > On Fri, Dec 16, 2016 at 11:30:03AM -0800, Rajat Jain wrote:
> >> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >> index ce22cef..beca4e9 100644
> >> --- a/drivers/bluetooth/btusb.c
> >> +++ b/drivers/bluetooth/btusb.c

> >> +static const struct of_device_id btusb_match_table[] = {
> >> +     { .compatible = "usb1286,204e" },
> >> +     { }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, btusb_match_table);
> >
> > You define a match table here, but you also define essentially same
> > table for Marvell-specific additions in patch 3. It looks like maybe
> > it's legal to have more than one OF table in a module? But it seems like
> > it would get confusing, besides being somewhat strange to maintain. It
> > might also produce duplicate 'modinfo' output.
> >
> > If you really want to independently opt into device-tree-specified
> > interrupts vs. Marvell-specific interrrupt configuration, then you
> > should probably just merge the latter into the former table, and
> > implement a Marvell/GPIO flag to stick in the .data field of this table.
> 
> The data we want to stick (The pin number on the chip) is board
> specific rather than being chip specific, and hence .data may not be
> useful here.

I just meant that if you conceptually wanted Marvell devices to "opt in"
to this, then you could turn the .data field into some kind of flag or
struct for describing capabilities (e.g., MARVELL_GPIO_WAKE or similar),
effectively merging the two tables instead of having two
mostly-overlapping tables.

But you could do the same by putting such flag(s) in the
{btusb,blacklist}_table[], or add such flag(s) later whenever there's a
Marvell device that doesn't support this feature.

> > Or it might be fine to drop one or both "match" checks. Particularly for
> > the Marvell-specific stuff, it's probably fair just to check if it has
> > an ->of_node and if 'id->driver_info & BTUSB_MARVELL'. Any other Marvell
> > device-specific quirks could probably be keyed off of the
> > (weirdly-named?) blacklist_table[], which already matches PID/VID.
> 
> Yup, I think I can remove the compatible string checks.
> 
> I'll be sending a V3.

Great!

Brian

^ permalink raw reply

* [PATCH v3 3/3] Bluetooth: btusb: Configure Marvell to use one of the pins for oob wakeup
From: Rajat Jain @ 2016-12-20  2:56 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Amitkumar Karwar, Wei-Ning Huang, Xinming Hu,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, Brian Norris,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Rajat Jain, rajatxjain-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1482202592-76314-1-git-send-email-rajatja-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

The Marvell devices may have many gpio pins, and hence for wakeup
on these out-of-band pins, the chip needs to be told which pin is
to be used for wakeup, using an hci command.

Thus, we read the pin number etc from the device tree node and send
a command to the chip.

Signed-off-by: Rajat Jain <rajatja-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
v3: * remove the Marvell specific id table and check
    * Add reference to marvell-bt-8xxx.txt in btusb.txt
    * Add "Reviewed-by" and "Acked-by"    
v2: Fix the binding document to specify to use "wakeup" interrupt-name

 Documentation/devicetree/bindings/net/btusb.txt    |  3 ++
 .../{marvell-bt-sd8xxx.txt => marvell-bt-8xxx.txt} | 46 +++++++++++++++----
 drivers/bluetooth/btusb.c                          | 51 ++++++++++++++++++++++
 3 files changed, 92 insertions(+), 8 deletions(-)
 rename Documentation/devicetree/bindings/net/{marvell-bt-sd8xxx.txt => marvell-bt-8xxx.txt} (50%)

diff --git a/Documentation/devicetree/bindings/net/btusb.txt b/Documentation/devicetree/bindings/net/btusb.txt
index 2c0355c..01fa2d4 100644
--- a/Documentation/devicetree/bindings/net/btusb.txt
+++ b/Documentation/devicetree/bindings/net/btusb.txt
@@ -10,6 +10,9 @@ Required properties:
 
 		  "usb1286,204e" (Marvell 8997)
 
+Also, vendors that use btusb may have device additional properties, e.g:
+Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
+
 Optional properties:
 
   - interrupt-parent: phandle of the parent interrupt controller
diff --git a/Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt b/Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
similarity index 50%
rename from Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt
rename to Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
index 6a9a63c..9be1059 100644
--- a/Documentation/devicetree/bindings/net/marvell-bt-sd8xxx.txt
+++ b/Documentation/devicetree/bindings/net/marvell-bt-8xxx.txt
@@ -1,16 +1,21 @@
-Marvell 8897/8997 (sd8897/sd8997) bluetooth SDIO devices
+Marvell 8897/8997 (sd8897/sd8997) bluetooth devices (SDIO or USB based)
 ------
+The 8997 devices supports multiple interfaces. When used on SDIO interfaces,
+the btmrvl driver is used and when used on USB interface, the btusb driver is
+used.
 
 Required properties:
 
   - compatible : should be one of the following:
-	* "marvell,sd8897-bt"
-	* "marvell,sd8997-bt"
+	* "marvell,sd8897-bt" (for SDIO)
+	* "marvell,sd8997-bt" (for SDIO)
+	* "usb1286,204e"      (for USB)
 
 Optional properties:
 
   - marvell,cal-data: Calibration data downloaded to the device during
 		      initialization. This is an array of 28 values(u8).
+		      This is only applicable to SDIO devices.
 
   - marvell,wakeup-pin: It represents wakeup pin number of the bluetooth chip.
 		        firmware will use the pin to wakeup host system (u16).
@@ -18,10 +23,15 @@ Optional properties:
 		      platform. The value will be configured to firmware. This
 		      is needed to work chip's sleep feature as expected (u16).
   - interrupt-parent: phandle of the parent interrupt controller
-  - interrupts : interrupt pin number to the cpu. Driver will request an irq based
-		 on this interrupt number. During system suspend, the irq will be
-		 enabled so that the bluetooth chip can wakeup host platform under
-		 certain condition. During system resume, the irq will be disabled
+  - interrupt-names: Used only for USB based devices (See below)
+  - interrupts : specifies the interrupt pin number to the cpu. For SDIO, the
+		 driver will use the first interrupt specified in the interrupt
+		 array. For USB based devices, the driver will use the interrupt
+		 named "wakeup" from the interrupt-names and interrupt arrays.
+		 The driver will request an irq based on this interrupt number.
+		 During system suspend, the irq will be enabled so that the
+		 bluetooth chip can wakeup host platform under certain
+		 conditions. During system resume, the irq will be disabled
 		 to make sure unnecessary interrupt is not received.
 
 Example:
@@ -29,7 +39,9 @@ Example:
 IRQ pin 119 is used as system wakeup source interrupt.
 wakeup pin 13 and gap 100ms are configured so that firmware can wakeup host
 using this device side pin and wakeup latency.
-calibration data is also available in below example.
+
+Example for SDIO device follows (calibration data is also available in
+below example).
 
 &mmc3 {
 	status = "okay";
@@ -54,3 +66,21 @@ calibration data is also available in below example.
 		marvell,wakeup-gap-ms = /bits/ 16 <0x64>;
 	};
 };
+
+Example for USB device:
+
+&usb_host1_ohci {
+    status = "okay";
+    #address-cells = <1>;
+    #size-cells = <0>;
+
+    mvl_bt1: bt@1 {
+	compatible = "usb1286,204e";
+	reg = <1>;
+	interrupt-parent = <&gpio0>;
+	interrupt-names = "wakeup";
+	interrupts = <119 IRQ_TYPE_LEVEL_LOW>;
+	marvell,wakeup-pin = /bits/ 16 <0x0d>;
+	marvell,wakeup-gap-ms = /bits/ 16 <0x64>;
+    };
+};
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index beca4e9..a5c2cf9 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2343,6 +2343,50 @@ static int btusb_shutdown_intel(struct hci_dev *hdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+/* Configure an out-of-band gpio as wake-up pin, if specified in device tree */
+static int marvell_config_oob_wake(struct hci_dev *hdev)
+{
+	struct sk_buff *skb;
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	struct device *dev = &data->udev->dev;
+	u16 pin, gap, opcode;
+	int ret;
+	u8 cmd[5];
+
+	/* Move on if no wakeup pin specified */
+	if (of_property_read_u16(dev->of_node, "marvell,wakeup-pin", &pin) ||
+	    of_property_read_u16(dev->of_node, "marvell,wakeup-gap-ms", &gap))
+		return 0;
+
+	/* Vendor specific command to configure a GPIO as wake-up pin */
+	opcode = hci_opcode_pack(0x3F, 0x59);
+	cmd[0] = opcode & 0xFF;
+	cmd[1] = opcode >> 8;
+	cmd[2] = 2; /* length of parameters that follow */
+	cmd[3] = pin;
+	cmd[4] = gap; /* time in ms, for which wakeup pin should be asserted */
+
+	skb = bt_skb_alloc(sizeof(cmd), GFP_KERNEL);
+	if (!skb) {
+		bt_dev_err(hdev, "%s: No memory\n", __func__);
+		return -ENOMEM;
+	}
+
+	memcpy(skb_put(skb, sizeof(cmd)), cmd, sizeof(cmd));
+	hci_skb_pkt_type(skb) = HCI_COMMAND_PKT;
+
+	ret = btusb_send_frame(hdev, skb);
+	if (ret) {
+		bt_dev_err(hdev, "%s: configuration failed\n", __func__);
+		kfree_skb(skb);
+		return ret;
+	}
+
+	return 0;
+}
+#endif
+
 static int btusb_set_bdaddr_marvell(struct hci_dev *hdev,
 				    const bdaddr_t *bdaddr)
 {
@@ -2917,6 +2961,13 @@ static int btusb_probe(struct usb_interface *intf,
 	err = btusb_config_oob_wake(hdev);
 	if (err)
 		goto out_free_dev;
+
+	/* Marvell devices may need a specific chip configuration */
+	if (id->driver_info & BTUSB_MARVELL && data->oob_wake_irq) {
+		err = marvell_config_oob_wake(hdev);
+		if (err)
+			goto out_free_dev;
+	}
 #endif
 	if (id->driver_info & BTUSB_CW6622)
 		set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks);
-- 
2.8.0.rc3.226.g39d4020

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v3 2/3] Bluetooth: btusb: Add out-of-band wakeup support
From: Rajat Jain @ 2016-12-20  2:56 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Amitkumar Karwar, Wei-Ning Huang, Xinming Hu,
	netdev, devicetree, linux-bluetooth, Brian Norris, linux-kernel
  Cc: Rajat Jain, rajatxjain
In-Reply-To: <1482202592-76314-1-git-send-email-rajatja@google.com>

Some onboard BT chips (e.g. Marvell 8997) contain a wakeup pin that
can be connected to a gpio on the CPU side, and can be used to wakeup
the host out-of-band. This can be useful in situations where the
in-band wakeup is not possible or not preferable (e.g. the in-band
wakeup may require the USB host controller to remain active, and
hence consuming more system power during system sleep).

The oob gpio interrupt to be used for wakeup on the CPU side, is
read from the device tree node, (using standard interrupt descriptors).
A devcie tree binding document is also added for the driver. The
compatible string is in compliance with
Documentation/devicetree/bindings/usb/usb-device.txt

Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
---
v3: Add Brian's "Reviewed-by"
v2: * Use interrupt-names ("wakeup") instead of assuming first interrupt.
    * Leave it on device tree to specify IRQ flags (level /edge triggered)
    * Mark the device as non wakeable on exit.

 Documentation/devicetree/bindings/net/btusb.txt | 40 ++++++++++++
 drivers/bluetooth/btusb.c                       | 84 +++++++++++++++++++++++++
 2 files changed, 124 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/btusb.txt

diff --git a/Documentation/devicetree/bindings/net/btusb.txt b/Documentation/devicetree/bindings/net/btusb.txt
new file mode 100644
index 0000000..2c0355c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/btusb.txt
@@ -0,0 +1,40 @@
+Generic Bluetooth controller over USB (btusb driver)
+---------------------------------------------------
+
+Required properties:
+
+  - compatible : should comply with the format "usbVID,PID" specified in
+		 Documentation/devicetree/bindings/usb/usb-device.txt
+		 At the time of writing, the only OF supported devices
+		 (more may be added later) are:
+
+		  "usb1286,204e" (Marvell 8997)
+
+Optional properties:
+
+  - interrupt-parent: phandle of the parent interrupt controller
+  - interrupt-names: (see below)
+  - interrupts : The interrupt specified by the name "wakeup" is the interrupt
+		 that shall be used for out-of-band wake-on-bt. Driver will
+		 request this interrupt for wakeup. During system suspend, the
+		 irq will be enabled so that the bluetooth chip can wakeup host
+		 platform out of band. During system resume, the irq will be
+		 disabled to make sure unnecessary interrupt is not received.
+
+Example:
+
+Following example uses irq pin number 3 of gpio0 for out of band wake-on-bt:
+
+&usb_host1_ehci {
+    status = "okay";
+    #address-cells = <1>;
+    #size-cells = <0>;
+
+    mvl_bt1: bt@1 {
+	compatible = "usb1286,204e";
+	reg = <1>;
+	interrupt-parent = <&gpio0>;
+	interrupt-name = "wakeup";
+	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+    };
+};
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index ce22cef..beca4e9 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -24,6 +24,8 @@
 #include <linux/module.h>
 #include <linux/usb.h>
 #include <linux/firmware.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
 #include <asm/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -369,6 +371,7 @@ static const struct usb_device_id blacklist_table[] = {
 #define BTUSB_BOOTING		9
 #define BTUSB_RESET_RESUME	10
 #define BTUSB_DIAG_RUNNING	11
+#define BTUSB_OOB_WAKE_DISABLED	12
 
 struct btusb_data {
 	struct hci_dev       *hdev;
@@ -416,6 +419,8 @@ struct btusb_data {
 	int (*recv_bulk)(struct btusb_data *data, void *buffer, int count);
 
 	int (*setup_on_usb)(struct hci_dev *hdev);
+
+	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
 };
 
 static inline void btusb_free_frags(struct btusb_data *data)
@@ -2728,6 +2733,65 @@ static int btusb_bcm_set_diag(struct hci_dev *hdev, bool enable)
 }
 #endif
 
+#ifdef CONFIG_PM
+static irqreturn_t btusb_oob_wake_handler(int irq, void *priv)
+{
+	struct btusb_data *data = priv;
+
+	/* Disable only if not already disabled (keep it balanced) */
+	if (!test_and_set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags)) {
+		disable_irq_nosync(irq);
+		disable_irq_wake(irq);
+	}
+	pm_wakeup_event(&data->udev->dev, 0);
+	return IRQ_HANDLED;
+}
+
+static const struct of_device_id btusb_match_table[] = {
+	{ .compatible = "usb1286,204e" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, btusb_match_table);
+
+/* Use an oob wakeup pin? */
+static int btusb_config_oob_wake(struct hci_dev *hdev)
+{
+	struct btusb_data *data = hci_get_drvdata(hdev);
+	struct device *dev = &data->udev->dev;
+	int irq, ret;
+
+	if (!of_match_device(btusb_match_table, dev))
+		return 0;
+
+	/* Move on if no IRQ specified */
+	irq = of_irq_get_byname(dev->of_node, "wakeup");
+	if (irq <= 0) {
+		bt_dev_dbg(hdev, "%s: no OOB Wakeup IRQ in DT", __func__);
+		return 0;
+	}
+
+	set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags);
+
+	ret = devm_request_irq(&hdev->dev, irq, btusb_oob_wake_handler,
+			       0, "OOB Wake-on-BT", data);
+	if (ret) {
+		bt_dev_err(hdev, "%s: IRQ request failed", __func__);
+		return ret;
+	}
+
+	ret = device_init_wakeup(dev, true);
+	if (ret) {
+		bt_dev_err(hdev, "%s: failed to init_wakeup\n", __func__);
+		return ret;
+	}
+
+	data->oob_wake_irq = irq;
+	disable_irq(irq);
+	bt_dev_info(hdev, "OOB Wake-on-BT configured at IRQ %u\n", irq);
+	return 0;
+}
+#endif
+
 static int btusb_probe(struct usb_interface *intf,
 		       const struct usb_device_id *id)
 {
@@ -2849,6 +2913,11 @@ static int btusb_probe(struct usb_interface *intf,
 	hdev->send   = btusb_send_frame;
 	hdev->notify = btusb_notify;
 
+#ifdef CONFIG_PM
+	err = btusb_config_oob_wake(hdev);
+	if (err)
+		goto out_free_dev;
+#endif
 	if (id->driver_info & BTUSB_CW6622)
 		set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks);
 
@@ -3061,6 +3130,9 @@ static void btusb_disconnect(struct usb_interface *intf)
 			usb_driver_release_interface(&btusb_driver, data->isoc);
 	}
 
+	if (data->oob_wake_irq)
+		device_init_wakeup(&data->udev->dev, false);
+
 	hci_free_dev(hdev);
 }
 
@@ -3089,6 +3161,12 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
 	btusb_stop_traffic(data);
 	usb_kill_anchored_urbs(&data->tx_anchor);
 
+	if (data->oob_wake_irq && device_may_wakeup(&data->udev->dev)) {
+		clear_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags);
+		enable_irq_wake(data->oob_wake_irq);
+		enable_irq(data->oob_wake_irq);
+	}
+
 	/* Optionally request a device reset on resume, but only when
 	 * wakeups are disabled. If wakeups are enabled we assume the
 	 * device will stay powered up throughout suspend.
@@ -3126,6 +3204,12 @@ static int btusb_resume(struct usb_interface *intf)
 	if (--data->suspend_count)
 		return 0;
 
+	/* Disable only if not already disabled (keep it balanced) */
+	if (!test_and_set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags)) {
+		disable_irq(data->oob_wake_irq);
+		disable_irq_wake(data->oob_wake_irq);
+	}
+
 	if (!test_bit(HCI_RUNNING, &hdev->flags))
 		goto done;
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related

* [PATCH v3 1/3] Bluetooth: btusb: Use an error label for error paths
From: Rajat Jain @ 2016-12-20  2:56 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Amitkumar Karwar, Wei-Ning Huang, Xinming Hu,
	netdev, devicetree, linux-bluetooth, Brian Norris, linux-kernel
  Cc: Rajat Jain, rajatxjain

Use a label to remove the repetetive cleanup, for error cases.

Signed-off-by: Rajat Jain <rajatja@google.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>
---
v3: Added Brian's "Reviewed-by"
v2: same as v1

 drivers/bluetooth/btusb.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 2f633df..ce22cef 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2991,18 +2991,15 @@ static int btusb_probe(struct usb_interface *intf,
 		err = usb_set_interface(data->udev, 0, 0);
 		if (err < 0) {
 			BT_ERR("failed to set interface 0, alt 0 %d", err);
-			hci_free_dev(hdev);
-			return err;
+			goto out_free_dev;
 		}
 	}
 
 	if (data->isoc) {
 		err = usb_driver_claim_interface(&btusb_driver,
 						 data->isoc, data);
-		if (err < 0) {
-			hci_free_dev(hdev);
-			return err;
-		}
+		if (err < 0)
+			goto out_free_dev;
 	}
 
 #ifdef CONFIG_BT_HCIBTUSB_BCM
@@ -3016,14 +3013,16 @@ static int btusb_probe(struct usb_interface *intf,
 #endif
 
 	err = hci_register_dev(hdev);
-	if (err < 0) {
-		hci_free_dev(hdev);
-		return err;
-	}
+	if (err < 0)
+		goto out_free_dev;
 
 	usb_set_intfdata(intf, data);
 
 	return 0;
+
+out_free_dev:
+	hci_free_dev(hdev);
+	return err;
 }
 
 static void btusb_disconnect(struct usb_interface *intf)
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related

* [PATCH 2/2] ARM: dts: hix5hd2: don't change the existing compatible string
From: Dongpo Li @ 2016-12-20  2:09 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
	salil.mehta-hv44wF8Li93QT0dZR+AlfA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	arnd-r2nGTMty4D4, andrew-g2DYL2Zd6BY
  Cc: xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q,
	benjamin.chenhao-C8/M+/jPZTeaMJb+Lgu22Q,
	caizhiyong-C8/M+/jPZTeaMJb+Lgu22Q, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Dongpo Li
In-Reply-To: <1482199769-106501-1-git-send-email-lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>

The SoC hix5hd2 compatible string has the suffix "-gmac" and
we should not change it.
We should only add the generic compatible string "hisi-gmac-v1".

Fixes: 0855950ba580 ("ARM: dts: hix5hd2: add gmac generic compatible and clock names")
Signed-off-by: Dongpo Li <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
---
 arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/hisi-x5hd2.dtsi b/arch/arm/boot/dts/hisi-x5hd2.dtsi
index 0da76c5..2d06f4c 100644
--- a/arch/arm/boot/dts/hisi-x5hd2.dtsi
+++ b/arch/arm/boot/dts/hisi-x5hd2.dtsi
@@ -436,7 +436,7 @@
 		};
 
 		gmac0: ethernet@1840000 {
-			compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
+			compatible = "hisilicon,hix5hd2-gmac", "hisilicon,hisi-gmac-v1";
 			reg = <0x1840000 0x1000>,<0x184300c 0x4>;
 			interrupts = <0 71 4>;
 			clocks = <&clock HIX5HD2_MAC0_CLK>;
@@ -445,7 +445,7 @@
 		};
 
 		gmac1: ethernet@1841000 {
-			compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
+			compatible = "hisilicon,hix5hd2-gmac", "hisilicon,hisi-gmac-v1";
 			reg = <0x1841000 0x1000>,<0x1843010 0x4>;
 			interrupts = <0 72 4>;
 			clocks = <&clock HIX5HD2_MAC1_CLK>;
-- 
2.8.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 1/2] net: hix5hd2_gmac: fix compatible strings name
From: Dongpo Li @ 2016-12-20  2:09 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, yisen.zhuang, salil.mehta, davem,
	arnd, andrew
  Cc: xuejiancheng, benjamin.chenhao, caizhiyong, netdev, devicetree,
	linux-kernel, Dongpo Li
In-Reply-To: <1482199769-106501-1-git-send-email-lidongpo@hisilicon.com>

The SoC hix5hd2 compatible string has the suffix "-gmac" and
we should not change its compatible string.
So we should name all the compatible string with the suffix "-gmac".
Creating a new name suffix "-gemac" is unnecessary.

We also add another SoC compatible string in dt binding documentation
and describe which generic version the SoC belongs to.

Fixes: d0fb6ba75dc0 ("net: hix5hd2_gmac: add generic compatible string")
Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
---
 .../devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt      | 13 ++++++++-----
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c               | 13 +++++++------
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
index 063c02d..eea73ad 100644
--- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
+++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
@@ -2,11 +2,14 @@ Hisilicon hix5hd2 gmac controller
 
 Required properties:
 - compatible: should contain one of the following SoC strings:
-	* "hisilicon,hix5hd2-gemac"
-	* "hisilicon,hi3798cv200-gemac"
+	* "hisilicon,hix5hd2-gmac"
+	* "hisilicon,hi3798cv200-gmac"
+	* "hisilicon,hi3516a-gmac"
 	and one of the following version string:
-	* "hisilicon,hisi-gemac-v1"
-	* "hisilicon,hisi-gemac-v2"
+	* "hisilicon,hisi-gmac-v1"
+	* "hisilicon,hisi-gmac-v2"
+  The version v1 includes SoCs hix5hd2.
+  The version v2 includes SoCs hi3798cv200, hi3516a.
 - reg: specifies base physical address(s) and size of the device registers.
   The first region is the MAC register base and size.
   The second region is external interface control register.
@@ -35,7 +38,7 @@ Required properties:
 
 Example:
 	gmac0: ethernet@f9840000 {
-		compatible = "hisilicon,hi3798cv200-gemac", "hisilicon,hisi-gemac-v2";
+		compatible = "hisilicon,hi3798cv200-gmac", "hisilicon,hisi-gmac-v2";
 		reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
 		interrupts = <0 71 4>;
 		#address-cells = <1>;
diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index ee7e9ce..418ca1f3 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -1316,10 +1316,11 @@ static int hix5hd2_dev_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id hix5hd2_of_match[] = {
-	{ .compatible = "hisilicon,hisi-gemac-v1", .data = (void *)GEMAC_V1 },
-	{ .compatible = "hisilicon,hisi-gemac-v2", .data = (void *)GEMAC_V2 },
-	{ .compatible = "hisilicon,hix5hd2-gemac", .data = (void *)GEMAC_V1 },
-	{ .compatible = "hisilicon,hi3798cv200-gemac", .data = (void *)GEMAC_V2 },
+	{ .compatible = "hisilicon,hisi-gmac-v1", .data = (void *)GEMAC_V1 },
+	{ .compatible = "hisilicon,hisi-gmac-v2", .data = (void *)GEMAC_V2 },
+	{ .compatible = "hisilicon,hix5hd2-gmac", .data = (void *)GEMAC_V1 },
+	{ .compatible = "hisilicon,hi3798cv200-gmac", .data = (void *)GEMAC_V2 },
+	{ .compatible = "hisilicon,hi3516a-gmac", .data = (void *)GEMAC_V2 },
 	{},
 };
 
@@ -1327,7 +1328,7 @@ MODULE_DEVICE_TABLE(of, hix5hd2_of_match);
 
 static struct platform_driver hix5hd2_dev_driver = {
 	.driver = {
-		.name = "hisi-gemac",
+		.name = "hisi-gmac",
 		.of_match_table = hix5hd2_of_match,
 	},
 	.probe = hix5hd2_dev_probe,
@@ -1338,4 +1339,4 @@ module_platform_driver(hix5hd2_dev_driver);
 
 MODULE_DESCRIPTION("HISILICON Gigabit Ethernet MAC driver");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:hisi-gemac");
+MODULE_ALIAS("platform:hisi-gmac");
-- 
2.8.2

^ permalink raw reply related

* [PATCH 0/2] net: hix5hd2_gmac: keep the compatible string not changed
From: Dongpo Li @ 2016-12-20  2:09 UTC (permalink / raw)
  To: robh+dt, mark.rutland, linux, yisen.zhuang, salil.mehta, davem,
	arnd, andrew
  Cc: xuejiancheng, benjamin.chenhao, caizhiyong, netdev, devicetree,
	linux-kernel, Dongpo Li

This patch series fix the patch:
d0fb6ba75dc0 ("net: hix5hd2_gmac: add generic compatible string")

The SoC hix5hd2 compatible string has the suffix "-gmac" and
we should not change its compatible string.
So we should name all the compatible string with the suffix "-gmac".
Creating a new name suffix "-gemac" is unnecessary.

Dongpo Li (2):
  net: hix5hd2_gmac: fix compatible strings name
  ARM: dts: hix5hd2: don't change the existing compatible string

 .../devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt      | 13 ++++++++-----
 arch/arm/boot/dts/hisi-x5hd2.dtsi                           |  4 ++--
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c               | 13 +++++++------
 3 files changed, 17 insertions(+), 13 deletions(-)

-- 
2.8.2

^ permalink raw reply

* Re: [PATCH v2 2/3] Bluetooth: btusb: Add out-of-band wakeup support
From: Rajat Jain @ 2016-12-20  1:46 UTC (permalink / raw)
  To: Brian Norris
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Amitkumar Karwar, Wei-Ning Huang, Xinming Hu,
	netdev, devicetree, linux-bluetooth, linux-kernel, Rajat Jain
In-Reply-To: <20161219231021.GA142074@google.com>

Hi Brian,


On Mon, Dec 19, 2016 at 3:10 PM, Brian Norris <briannorris@chromium.org> wrote:
> Hi Rajat,
>
> On Fri, Dec 16, 2016 at 11:30:03AM -0800, Rajat Jain wrote:
>> Some onboard BT chips (e.g. Marvell 8997) contain a wakeup pin that
>> can be connected to a gpio on the CPU side, and can be used to wakeup
>> the host out-of-band. This can be useful in situations where the
>> in-band wakeup is not possible or not preferable (e.g. the in-band
>> wakeup may require the USB host controller to remain active, and
>> hence consuming more system power during system sleep).
>>
>> The oob gpio interrupt to be used for wakeup on the CPU side, is
>> read from the device tree node, (using standard interrupt descriptors).
>> A devcie tree binding document is also added for the driver. The
>> compatible string is in compliance with
>> Documentation/devicetree/bindings/usb/usb-device.txt
>>
>> Signed-off-by: Rajat Jain <rajatja@google.com>
>
> A few small comments below, but other than those, for the whole series:
>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
>
>> ---
>> v2: * Use interrupt-names ("wakeup") instead of assuming first interrupt.
>>     * Leave it on device tree to specify IRQ flags (level /edge triggered)
>>     * Mark the device as non wakeable on exit.
>>
>>  Documentation/devicetree/bindings/net/btusb.txt | 40 ++++++++++++
>>  drivers/bluetooth/btusb.c                       | 84 +++++++++++++++++++++++++
>>  2 files changed, 124 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/btusb.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/btusb.txt b/Documentation/devicetree/bindings/net/btusb.txt
>> new file mode 100644
>> index 0000000..2c0355c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/btusb.txt
>> @@ -0,0 +1,40 @@
>> +Generic Bluetooth controller over USB (btusb driver)
>> +---------------------------------------------------
>> +
>> +Required properties:
>> +
>> +  - compatible : should comply with the format "usbVID,PID" specified in
>> +              Documentation/devicetree/bindings/usb/usb-device.txt
>> +              At the time of writing, the only OF supported devices
>> +              (more may be added later) are:
>> +
>> +               "usb1286,204e" (Marvell 8997)
>
> I don't know if anyone cares about these sort of organizational aspects,
> but in patch 3 you're extending this binding with device-specific
> Marvell bindings. Seems like it'd be good to have some clear way to
> correlate those 2. e.g., add a reference to marvell-bt-sd8xxx.txt (or
> marvell-bt-8xxx.txt, once you rename it) in patch 3.

Good idea, I'll do it. (I'll add a reference to marvell-bt-8xxx.txt in
btusb.txt).

>
>> +
>> +Optional properties:
>> +
>> +  - interrupt-parent: phandle of the parent interrupt controller
>> +  - interrupt-names: (see below)
>> +  - interrupts : The interrupt specified by the name "wakeup" is the interrupt
>> +              that shall be used for out-of-band wake-on-bt. Driver will
>> +              request this interrupt for wakeup. During system suspend, the
>> +              irq will be enabled so that the bluetooth chip can wakeup host
>> +              platform out of band. During system resume, the irq will be
>> +              disabled to make sure unnecessary interrupt is not received.
>> +
>> +Example:
>> +
>> +Following example uses irq pin number 3 of gpio0 for out of band wake-on-bt:
>> +
>> +&usb_host1_ehci {
>> +    status = "okay";
>> +    #address-cells = <1>;
>> +    #size-cells = <0>;
>> +
>> +    mvl_bt1: bt@1 {
>> +     compatible = "usb1286,204e";
>> +     reg = <1>;
>> +     interrupt-parent = <&gpio0>;
>> +     interrupt-name = "wakeup";
>> +     interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
>> +    };
>> +};
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index ce22cef..beca4e9 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -24,6 +24,8 @@
>>  #include <linux/module.h>
>>  #include <linux/usb.h>
>>  #include <linux/firmware.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_irq.h>
>>  #include <asm/unaligned.h>
>>
>>  #include <net/bluetooth/bluetooth.h>
>> @@ -369,6 +371,7 @@ static const struct usb_device_id blacklist_table[] = {
>>  #define BTUSB_BOOTING                9
>>  #define BTUSB_RESET_RESUME   10
>>  #define BTUSB_DIAG_RUNNING   11
>> +#define BTUSB_OOB_WAKE_DISABLED      12
>>
>>  struct btusb_data {
>>       struct hci_dev       *hdev;
>> @@ -416,6 +419,8 @@ struct btusb_data {
>>       int (*recv_bulk)(struct btusb_data *data, void *buffer, int count);
>>
>>       int (*setup_on_usb)(struct hci_dev *hdev);
>> +
>> +     int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
>>  };
>>
>>  static inline void btusb_free_frags(struct btusb_data *data)
>> @@ -2728,6 +2733,65 @@ static int btusb_bcm_set_diag(struct hci_dev *hdev, bool enable)
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_PM
>> +static irqreturn_t btusb_oob_wake_handler(int irq, void *priv)
>> +{
>> +     struct btusb_data *data = priv;
>> +
>> +     /* Disable only if not already disabled (keep it balanced) */
>> +     if (!test_and_set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags)) {
>> +             disable_irq_nosync(irq);
>> +             disable_irq_wake(irq);
>> +     }
>> +     pm_wakeup_event(&data->udev->dev, 0);
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static const struct of_device_id btusb_match_table[] = {
>> +     { .compatible = "usb1286,204e" },
>> +     { }
>> +};
>> +MODULE_DEVICE_TABLE(of, btusb_match_table);
>
> You define a match table here, but you also define essentially same
> table for Marvell-specific additions in patch 3. It looks like maybe
> it's legal to have more than one OF table in a module? But it seems like
> it would get confusing, besides being somewhat strange to maintain. It
> might also produce duplicate 'modinfo' output.
>
> If you really want to independently opt into device-tree-specified
> interrupts vs. Marvell-specific interrrupt configuration, then you
> should probably just merge the latter into the former table, and
> implement a Marvell/GPIO flag to stick in the .data field of this table.

The data we want to stick (The pin number on the chip) is board
specific rather than being chip specific, and hence .data may not be
useful here.

>
> Or it might be fine to drop one or both "match" checks. Particularly for
> the Marvell-specific stuff, it's probably fair just to check if it has
> an ->of_node and if 'id->driver_info & BTUSB_MARVELL'. Any other Marvell
> device-specific quirks could probably be keyed off of the
> (weirdly-named?) blacklist_table[], which already matches PID/VID.

Yup, I think I can remove the compatible string checks.

I'll be sending a V3.

>
>> +
>> +/* Use an oob wakeup pin? */
>> +static int btusb_config_oob_wake(struct hci_dev *hdev)
>> +{
>> +     struct btusb_data *data = hci_get_drvdata(hdev);
>> +     struct device *dev = &data->udev->dev;
>> +     int irq, ret;
>> +
>> +     if (!of_match_device(btusb_match_table, dev))
>> +             return 0;
>> +
>> +     /* Move on if no IRQ specified */
>> +     irq = of_irq_get_byname(dev->of_node, "wakeup");
>> +     if (irq <= 0) {
>> +             bt_dev_dbg(hdev, "%s: no OOB Wakeup IRQ in DT", __func__);
>> +             return 0;
>> +     }
>> +
>> +     set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags);
>> +
>> +     ret = devm_request_irq(&hdev->dev, irq, btusb_oob_wake_handler,
>> +                            0, "OOB Wake-on-BT", data);
>> +     if (ret) {
>> +             bt_dev_err(hdev, "%s: IRQ request failed", __func__);
>> +             return ret;
>> +     }
>> +
>> +     ret = device_init_wakeup(dev, true);
>> +     if (ret) {
>> +             bt_dev_err(hdev, "%s: failed to init_wakeup\n", __func__);
>> +             return ret;
>> +     }
>> +
>> +     data->oob_wake_irq = irq;
>> +     disable_irq(irq);
>> +     bt_dev_info(hdev, "OOB Wake-on-BT configured at IRQ %u\n", irq);
>> +     return 0;
>> +}
>> +#endif
>> +
>>  static int btusb_probe(struct usb_interface *intf,
>>                      const struct usb_device_id *id)
>>  {
>> @@ -2849,6 +2913,11 @@ static int btusb_probe(struct usb_interface *intf,
>>       hdev->send   = btusb_send_frame;
>>       hdev->notify = btusb_notify;
>>
>> +#ifdef CONFIG_PM
>> +     err = btusb_config_oob_wake(hdev);
>> +     if (err)
>> +             goto out_free_dev;
>> +#endif
>>       if (id->driver_info & BTUSB_CW6622)
>>               set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks);
>>
>> @@ -3061,6 +3130,9 @@ static void btusb_disconnect(struct usb_interface *intf)
>>                       usb_driver_release_interface(&btusb_driver, data->isoc);
>>       }
>>
>> +     if (data->oob_wake_irq)
>> +             device_init_wakeup(&data->udev->dev, false);
>> +
>>       hci_free_dev(hdev);
>>  }
>>
>> @@ -3089,6 +3161,12 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
>>       btusb_stop_traffic(data);
>>       usb_kill_anchored_urbs(&data->tx_anchor);
>>
>> +     if (data->oob_wake_irq && device_may_wakeup(&data->udev->dev)) {
>> +             clear_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags);
>> +             enable_irq_wake(data->oob_wake_irq);
>> +             enable_irq(data->oob_wake_irq);
>> +     }
>> +
>>       /* Optionally request a device reset on resume, but only when
>>        * wakeups are disabled. If wakeups are enabled we assume the
>>        * device will stay powered up throughout suspend.
>> @@ -3126,6 +3204,12 @@ static int btusb_resume(struct usb_interface *intf)
>>       if (--data->suspend_count)
>
> Not related to your patch exactly, but isn't this 'suspend_count' stuff
> useless? I'd send a patch, but it might conflict with yours. Just wanted
> to note it and see if I'm crazy...
>
> Brian
>
>>               return 0;
>>
>> +     /* Disable only if not already disabled (keep it balanced) */
>> +     if (!test_and_set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags)) {
>> +             disable_irq(data->oob_wake_irq);
>> +             disable_irq_wake(data->oob_wake_irq);
>> +     }
>> +
>>       if (!test_bit(HCI_RUNNING, &hdev->flags))
>>               goto done;
>>
>> --
>> 2.8.0.rc3.226.g39d4020
>>

^ permalink raw reply

* Re: [PATCH v2 1/4] net: hix5hd2_gmac: add generic compatible string
From: Dongpo Li @ 2016-12-20  1:35 UTC (permalink / raw)
  To: Rob Herring
  Cc: David Miller, Mark Rutland, Michael Turquette, Stephen Boyd,
	Russell King, Zhangfei Gao, Yisen Zhuang, salil.mehta,
	Arnd Bergmann, Andrew Lunn, Jiancheng Xue, benjamin.chenhao,
	caizhiyong, netdev, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+Su84DonQbh2KZ2o8JQ0Mqarag60Oq=Dby3y8Doyozxg@mail.gmail.com>



On 2016/12/20 0:04, Rob Herring wrote:
> On Mon, Dec 19, 2016 at 2:14 AM, Dongpo Li <lidongpo@hisilicon.com> wrote:
>> Hi Rob and David,
>>
>> On 2016/12/12 22:21, Rob Herring wrote:
>>> On Mon, Dec 12, 2016 at 5:16 AM, Dongpo Li <lidongpo@hisilicon.com> wrote:
>>>> Hi Rob,
>>>>
>>>> On 2016/12/10 6:35, Rob Herring wrote:
>>>>> On Mon, Dec 05, 2016 at 09:27:58PM +0800, Dongpo Li wrote:
> 
> [...]
> 
>>>>>> @@ -20,7 +25,7 @@ Required properties:
>>>>>>
>>>>>>  Example:
>>>>>>      gmac0: ethernet@f9840000 {
>>>>>> -            compatible = "hisilicon,hix5hd2-gmac";
>>>>>> +            compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
>>>>>
>>>>> You can't just change compatible strings.
>>>>>
>>>> Okay, maybe I should name all the compatible string with the suffix "-gmac" instead of
>>>> "-gemac". This can keep the compatible strings with the same suffix. Is this okay?
>>>> Can I just add the generic compatible string without changing the SoCs compatible string?
>>>> Like following:
>>>>         gmac0: ethernet@f9840000 {
>>>>  -              compatible = "hisilicon,hix5hd2-gmac";
>>>>  +              compatible = "hisilicon,hix5hd2-gmac", "hisilicon,hisi-gmac-v1";
>>>
>>> Yes, this is fine.
>>
>> As the patch series have been applied to net-next branch,
>> in which way should I commit this compatible fix?
>> Should I send a new patch fixing this compatible string error with "Fixes: xxx"?
>> Looking forward to your reply!
> 
> Yes to both.

Okay, thanks for your reply!
I will send the fix patch series as soon as possible.


    Regards,
    Dongpo

.

^ permalink raw reply

* Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock
From: Geoff Lansberry @ 2016-12-19 23:23 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-wireless, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, mark.rutland-5wv7dgnIgG8,
	netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mark Greer, Justin Bronder
In-Reply-To: <20161219223111.itu77g7wsqtj6cvs@rob-hp-laptop>

I can make that change, however, I worry that it may be a bit
misleading, since there are only two supported clock frequencies, but
a number like that to me implies that it could be set to any number
you want.   I'm new at this, and so I'll go ahead and change it as you
request, but I'd like to hear your thoughts on my concern.

Thanks
Geoff
Geoff Lansberry


Engineering Guy
Kuvée, Inc
125 Kingston St., 3rd Floor
Boston, MA 02111
1-617-290-1118 (m)
geoff.lansberry (skype)
http://www.kuvee.com



On Mon, Dec 19, 2016 at 5:31 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
>> From: Geoff Lansberry <geoff-R+k406RtEhcAvxtiuMwx3w@public.gmane.org>
>>
>> ---
>>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  3 ++
>>  drivers/nfc/trf7970a.c                             | 42 ++++++++++++++++------
>>  2 files changed, 34 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> index 32b35a0..9dda879 100644
>> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
>> @@ -21,6 +21,8 @@ Optional SoC Specific Properties:
>>  - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
>>    where an extra byte is returned by Read Multiple Block commands issued
>>    to Type 5 tags.
>> +- crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
>> +
>
> Can't you use 'clock-frequency = "27000000";'?
>
>>
>>  Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>>
>> @@ -43,6 +45,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>>               irq-status-read-quirk;
>>               en2-rf-quirk;
>>               t5t-rmb-extra-byte-quirk;
>> +             crystal_27mhz;
>>               status = "okay";
>>       };
>>  };
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 2/3] Bluetooth: btusb: Add out-of-band wakeup support
From: Brian Norris @ 2016-12-19 23:10 UTC (permalink / raw)
  To: Rajat Jain
  Cc: Rob Herring, Mark Rutland, Marcel Holtmann, Gustavo Padovan,
	Johan Hedberg, Amitkumar Karwar, Wei-Ning Huang, Xinming Hu,
	netdev, devicetree, linux-bluetooth, linux-kernel, rajatxjain
In-Reply-To: <1481916604-114279-2-git-send-email-rajatja@google.com>

Hi Rajat,

On Fri, Dec 16, 2016 at 11:30:03AM -0800, Rajat Jain wrote:
> Some onboard BT chips (e.g. Marvell 8997) contain a wakeup pin that
> can be connected to a gpio on the CPU side, and can be used to wakeup
> the host out-of-band. This can be useful in situations where the
> in-band wakeup is not possible or not preferable (e.g. the in-band
> wakeup may require the USB host controller to remain active, and
> hence consuming more system power during system sleep).
> 
> The oob gpio interrupt to be used for wakeup on the CPU side, is
> read from the device tree node, (using standard interrupt descriptors).
> A devcie tree binding document is also added for the driver. The
> compatible string is in compliance with
> Documentation/devicetree/bindings/usb/usb-device.txt
> 
> Signed-off-by: Rajat Jain <rajatja@google.com>

A few small comments below, but other than those, for the whole series:

Reviewed-by: Brian Norris <briannorris@chromium.org>

> ---
> v2: * Use interrupt-names ("wakeup") instead of assuming first interrupt.
>     * Leave it on device tree to specify IRQ flags (level /edge triggered)
>     * Mark the device as non wakeable on exit.
> 
>  Documentation/devicetree/bindings/net/btusb.txt | 40 ++++++++++++
>  drivers/bluetooth/btusb.c                       | 84 +++++++++++++++++++++++++
>  2 files changed, 124 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/btusb.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/btusb.txt b/Documentation/devicetree/bindings/net/btusb.txt
> new file mode 100644
> index 0000000..2c0355c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/btusb.txt
> @@ -0,0 +1,40 @@
> +Generic Bluetooth controller over USB (btusb driver)
> +---------------------------------------------------
> +
> +Required properties:
> +
> +  - compatible : should comply with the format "usbVID,PID" specified in
> +		 Documentation/devicetree/bindings/usb/usb-device.txt
> +		 At the time of writing, the only OF supported devices
> +		 (more may be added later) are:
> +
> +		  "usb1286,204e" (Marvell 8997)

I don't know if anyone cares about these sort of organizational aspects,
but in patch 3 you're extending this binding with device-specific
Marvell bindings. Seems like it'd be good to have some clear way to
correlate those 2. e.g., add a reference to marvell-bt-sd8xxx.txt (or
marvell-bt-8xxx.txt, once you rename it) in patch 3.

> +
> +Optional properties:
> +
> +  - interrupt-parent: phandle of the parent interrupt controller
> +  - interrupt-names: (see below)
> +  - interrupts : The interrupt specified by the name "wakeup" is the interrupt
> +		 that shall be used for out-of-band wake-on-bt. Driver will
> +		 request this interrupt for wakeup. During system suspend, the
> +		 irq will be enabled so that the bluetooth chip can wakeup host
> +		 platform out of band. During system resume, the irq will be
> +		 disabled to make sure unnecessary interrupt is not received.
> +
> +Example:
> +
> +Following example uses irq pin number 3 of gpio0 for out of band wake-on-bt:
> +
> +&usb_host1_ehci {
> +    status = "okay";
> +    #address-cells = <1>;
> +    #size-cells = <0>;
> +
> +    mvl_bt1: bt@1 {
> +	compatible = "usb1286,204e";
> +	reg = <1>;
> +	interrupt-parent = <&gpio0>;
> +	interrupt-name = "wakeup";
> +	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +    };
> +};
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index ce22cef..beca4e9 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -24,6 +24,8 @@
>  #include <linux/module.h>
>  #include <linux/usb.h>
>  #include <linux/firmware.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
>  #include <asm/unaligned.h>
>  
>  #include <net/bluetooth/bluetooth.h>
> @@ -369,6 +371,7 @@ static const struct usb_device_id blacklist_table[] = {
>  #define BTUSB_BOOTING		9
>  #define BTUSB_RESET_RESUME	10
>  #define BTUSB_DIAG_RUNNING	11
> +#define BTUSB_OOB_WAKE_DISABLED	12
>  
>  struct btusb_data {
>  	struct hci_dev       *hdev;
> @@ -416,6 +419,8 @@ struct btusb_data {
>  	int (*recv_bulk)(struct btusb_data *data, void *buffer, int count);
>  
>  	int (*setup_on_usb)(struct hci_dev *hdev);
> +
> +	int oob_wake_irq;   /* irq for out-of-band wake-on-bt */
>  };
>  
>  static inline void btusb_free_frags(struct btusb_data *data)
> @@ -2728,6 +2733,65 @@ static int btusb_bcm_set_diag(struct hci_dev *hdev, bool enable)
>  }
>  #endif
>  
> +#ifdef CONFIG_PM
> +static irqreturn_t btusb_oob_wake_handler(int irq, void *priv)
> +{
> +	struct btusb_data *data = priv;
> +
> +	/* Disable only if not already disabled (keep it balanced) */
> +	if (!test_and_set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags)) {
> +		disable_irq_nosync(irq);
> +		disable_irq_wake(irq);
> +	}
> +	pm_wakeup_event(&data->udev->dev, 0);
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct of_device_id btusb_match_table[] = {
> +	{ .compatible = "usb1286,204e" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, btusb_match_table);

You define a match table here, but you also define essentially same
table for Marvell-specific additions in patch 3. It looks like maybe
it's legal to have more than one OF table in a module? But it seems like
it would get confusing, besides being somewhat strange to maintain. It
might also produce duplicate 'modinfo' output.

If you really want to independently opt into device-tree-specified
interrupts vs. Marvell-specific interrrupt configuration, then you
should probably just merge the latter into the former table, and
implement a Marvell/GPIO flag to stick in the .data field of this table.

Or it might be fine to drop one or both "match" checks. Particularly for
the Marvell-specific stuff, it's probably fair just to check if it has
an ->of_node and if 'id->driver_info & BTUSB_MARVELL'. Any other Marvell
device-specific quirks could probably be keyed off of the
(weirdly-named?) blacklist_table[], which already matches PID/VID.

> +
> +/* Use an oob wakeup pin? */
> +static int btusb_config_oob_wake(struct hci_dev *hdev)
> +{
> +	struct btusb_data *data = hci_get_drvdata(hdev);
> +	struct device *dev = &data->udev->dev;
> +	int irq, ret;
> +
> +	if (!of_match_device(btusb_match_table, dev))
> +		return 0;
> +
> +	/* Move on if no IRQ specified */
> +	irq = of_irq_get_byname(dev->of_node, "wakeup");
> +	if (irq <= 0) {
> +		bt_dev_dbg(hdev, "%s: no OOB Wakeup IRQ in DT", __func__);
> +		return 0;
> +	}
> +
> +	set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags);
> +
> +	ret = devm_request_irq(&hdev->dev, irq, btusb_oob_wake_handler,
> +			       0, "OOB Wake-on-BT", data);
> +	if (ret) {
> +		bt_dev_err(hdev, "%s: IRQ request failed", __func__);
> +		return ret;
> +	}
> +
> +	ret = device_init_wakeup(dev, true);
> +	if (ret) {
> +		bt_dev_err(hdev, "%s: failed to init_wakeup\n", __func__);
> +		return ret;
> +	}
> +
> +	data->oob_wake_irq = irq;
> +	disable_irq(irq);
> +	bt_dev_info(hdev, "OOB Wake-on-BT configured at IRQ %u\n", irq);
> +	return 0;
> +}
> +#endif
> +
>  static int btusb_probe(struct usb_interface *intf,
>  		       const struct usb_device_id *id)
>  {
> @@ -2849,6 +2913,11 @@ static int btusb_probe(struct usb_interface *intf,
>  	hdev->send   = btusb_send_frame;
>  	hdev->notify = btusb_notify;
>  
> +#ifdef CONFIG_PM
> +	err = btusb_config_oob_wake(hdev);
> +	if (err)
> +		goto out_free_dev;
> +#endif
>  	if (id->driver_info & BTUSB_CW6622)
>  		set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks);
>  
> @@ -3061,6 +3130,9 @@ static void btusb_disconnect(struct usb_interface *intf)
>  			usb_driver_release_interface(&btusb_driver, data->isoc);
>  	}
>  
> +	if (data->oob_wake_irq)
> +		device_init_wakeup(&data->udev->dev, false);
> +
>  	hci_free_dev(hdev);
>  }
>  
> @@ -3089,6 +3161,12 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
>  	btusb_stop_traffic(data);
>  	usb_kill_anchored_urbs(&data->tx_anchor);
>  
> +	if (data->oob_wake_irq && device_may_wakeup(&data->udev->dev)) {
> +		clear_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags);
> +		enable_irq_wake(data->oob_wake_irq);
> +		enable_irq(data->oob_wake_irq);
> +	}
> +
>  	/* Optionally request a device reset on resume, but only when
>  	 * wakeups are disabled. If wakeups are enabled we assume the
>  	 * device will stay powered up throughout suspend.
> @@ -3126,6 +3204,12 @@ static int btusb_resume(struct usb_interface *intf)
>  	if (--data->suspend_count)

Not related to your patch exactly, but isn't this 'suspend_count' stuff
useless? I'd send a patch, but it might conflict with yours. Just wanted
to note it and see if I'm crazy...

Brian

>  		return 0;
>  
> +	/* Disable only if not already disabled (keep it balanced) */
> +	if (!test_and_set_bit(BTUSB_OOB_WAKE_DISABLED, &data->flags)) {
> +		disable_irq(data->oob_wake_irq);
> +		disable_irq_wake(data->oob_wake_irq);
> +	}
> +
>  	if (!test_bit(HCI_RUNNING, &hdev->flags))
>  		goto done;
>  
> -- 
> 2.8.0.rc3.226.g39d4020
> 

^ permalink raw reply

* Re: [PATCHv2 2/8] dt-bindings: document the STM32 RTC bindings
From: Rob Herring @ 2016-12-19 23:10 UTC (permalink / raw)
  To: Amelie Delaunay
  Cc: Alessandro Zummo, Alexandre Belloni, Mark Rutland,
	Maxime Coquelin, Alexandre Torgue, Russell King,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Gabriel Fernandez
In-Reply-To: <1481878257-29163-3-git-send-email-amelie.delaunay-qxv4g6HH51o@public.gmane.org>

On Fri, Dec 16, 2016 at 09:50:51AM +0100, Amelie Delaunay wrote:
> This patch adds documentation of device tree bindings for the STM32 RTC.
> 
> Signed-off-by: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
> ---
>  .../devicetree/bindings/rtc/st,stm32-rtc.txt       | 27 ++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: [PATCH 1/2] document: dt: add binding for Hi3660 SoC
From: Rob Herring @ 2016-12-19 23:09 UTC (permalink / raw)
  To: Chen Feng
  Cc: xuwei5, mark.rutland, devicetree, linux-kernel, xuyiping,
	suzhuangluan, haojian.zhuang, guodong.xu, dan.zhao, saberlily.xia,
	puck.chen, oliver.fu, xuhongtao8
In-Reply-To: <1481874627-79760-1-git-send-email-puck.chen@hisilicon.com>

On Fri, Dec 16, 2016 at 03:50:26PM +0800, Chen Feng wrote:
> Add binding for hisilicon Hi3660 SoC and HiKey960 Board.
> 
> Signed-off-by: Chen Feng <puck.chen@hisilicon.com>
> ---
>  Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt | 4 ++++
>  1 file changed, 4 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH] Docs: dt: Be explicit and consistent in reference to IOMMU specifiers
From: Rob Herring @ 2016-12-19 23:06 UTC (permalink / raw)
  To: Stuart Yoder
  Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	will.deacon-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
In-Reply-To: <1481847373-2602-1-git-send-email-stuart.yoder-3arQi8VN3Tc@public.gmane.org>

On Thu, Dec 15, 2016 at 06:16:13PM -0600, Stuart Yoder wrote:
> The generic IOMMU binding says that the meaning of an 'IOMMU specifier'
> is defined by the binding of a specific SMMU.  The ARM SMMU binding
> never explicitly uses the term 'specifier' at all.  Update implicit
> references to use the explicit term.
> 
> In the iommu-map binding change references to iommu-specifier to
> "IOMMU specifier" so we are 100% consistent everywhere with terminology
> and capitalization.
> 
> Signed-off-by: Stuart Yoder <stuart.yoder-3arQi8VN3Tc@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/iommu/arm,smmu.txt | 10 +++++-----
>  Documentation/devicetree/bindings/pci/pci-iommu.txt  |  6 +++---
>  2 files changed, 8 insertions(+), 8 deletions(-)

Applied, thanks.

Rob

> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index e862d148..6cdf32d 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -36,15 +36,15 @@ conditions.
>                    combined interrupt, it must be listed multiple times.
>  
>  - #iommu-cells  : See Documentation/devicetree/bindings/iommu/iommu.txt
> -                  for details. With a value of 1, each "iommus" entry
> +                  for details. With a value of 1, each IOMMU specifier
>                    represents a distinct stream ID emitted by that device
>                    into the relevant SMMU.
>  
>                    SMMUs with stream matching support and complex masters
> -                  may use a value of 2, where the second cell represents
> -                  an SMR mask to combine with the ID in the first cell.
> -                  Care must be taken to ensure the set of matched IDs
> -                  does not result in conflicts.
> +                  may use a value of 2, where the second cell of the
> +                  IOMMU specifier represents an SMR mask to combine with
> +                  the ID in the first cell.  Care must be taken to ensure
> +                  the set of matched IDs does not result in conflicts.
>  
>  ** System MMU optional properties:
>  
> diff --git a/Documentation/devicetree/bindings/pci/pci-iommu.txt b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> index 56c8296..0def586 100644
> --- a/Documentation/devicetree/bindings/pci/pci-iommu.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> @@ -32,17 +32,17 @@ PCI root complex
>  Optional properties
>  -------------------
>  
> -- iommu-map: Maps a Requester ID to an IOMMU and associated iommu-specifier
> +- iommu-map: Maps a Requester ID to an IOMMU and associated IOMMU specifier
>    data.
>  
>    The property is an arbitrary number of tuples of
>    (rid-base,iommu,iommu-base,length).
>  
>    Any RID r in the interval [rid-base, rid-base + length) is associated with
> -  the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
> +  the listed IOMMU, with the IOMMU specifier (r - rid-base + iommu-base).
>  
>  - iommu-map-mask: A mask to be applied to each Requester ID prior to being
> -  mapped to an iommu-specifier per the iommu-map property.
> +  mapped to an IOMMU specifier per the iommu-map property.
>  
>  
>  Example (1)
> -- 
> 1.9.0
> 

^ permalink raw reply

* Re: [PATCH 1/3 v2] iio: adc: add device tree bindings for Qualcomm PM8xxx ADCs
From: Rob Herring @ 2016-12-19 22:50 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Ivan T . Ivanov, Andy Gross,
	Bjorn Andersson, Stephen Boyd, Srinivas Kandagatla
In-Reply-To: <1481842089-13999-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On Thu, Dec 15, 2016 at 11:48:09PM +0100, Linus Walleij wrote:
> This adds the device tree bindings for the Qualcomm PM8xxx
> ADCs. This is based on the existing DT bindings for the
> SPMI ADC so there are hopefully no controversial features.
> 
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Ivan T. Ivanov <iivanov.xz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Andy Gross <andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Bjorn Andersson <bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> ChangeLog v1->v2:
> - Spelling fixes
> ---
>  .../bindings/iio/adc/qcom,pm8xxx-xoadc.txt         | 160 +++++++++++++++++++++
>  1 file changed, 160 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt b/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> new file mode 100644
> index 000000000000..3c6bca5b4edf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/qcom,pm8xxx-xoadc.txt
> @@ -0,0 +1,160 @@
> +Qualcomm's PM8xxx voltage XOADC
> +
> +The Qualcomm PM8xxx PMICs contain a HK/XO ADC (Housekeeping/Crystal
> +oscillator ADC) encompassing PM8018, PM8038, PM8058, PM8917 and PM8921.
> +
> +Required properties:
> +
> +- compatible: should be one of:
> +  "qcom,pm8018-adc"
> +  "qcom,pm8038-adc"
> +  "qcom,pm8058-adc"
> +  "qcom,pm8917-adc"
> +  "qcom,pm8921-adc"
> +
> +- reg: should contain the ADC base address in the PMIC, typically
> +  0x197.
> +
> +The following required properties are standard for IO channels, see
> +iio-bindings.txt for more details:
> +
> +- #address-cells: should be set to <1>
> +
> +- #size-cells: should be set to <0>
> +
> +- #io-channel-cells: should be set to <1>
> +
> +- interrupts: should refer to the parent PMIC interrupt controller
> +  and reference the proper ADC interrupt.
> +
> +Required subnodes:
> +
> +The ADC channels are configured as subnodes of the ADC. Since some of
> +them are used for calibrating the ADC, these nodes are compulsory:
> +
> +ref_625mv {

ref@c

> +	reg = <0x0c>;
> +};
> +
> +ref_1250mv {
> +	reg = <0x0d>;
> +};
> +
> +ref_muxoff {
> +	reg = <0x0f>;
> +};
> +
> +These three nodes are used for absolute and ratiometric calibration
> +and only need to have these reg values: they are by hardware definition
> +1:1 ratio converters that sample 625, 1250 and 0 milliV and create
> +an interpolation calibration for all other ADCs.
> +
> +Optional subnodes: any channels other than channel 0x0c, 0x0d and
> +0x0f are optional.
> +
> +Required channel node properties:
> +
> +- reg: should contain the hardware channel number in the range
> +  0 .. 0x0f (4 bits). The hardware only supports 16 channels.
> +
> +Optional channel node properties:
> +
> +- qcom,decimation:
> +  Value type: <u32>
> +  Definition: This parameter is used to decrease the ADC sampling rate.
> +          Quicker measurements can be made by reducing the decimation ratio.
> +          Valid values are 512, 1024, 2048, 4096.
> +          If the property is not found, a default value of 512 will be used.
> +
> +- qcom,ratiometric:
> +  Value type: <empty>
> +  Definition: Channel calibration type. If this property is specified
> +          VADC will use the VDD reference (1.8V) and GND for channel
> +          calibration. If the property is not found, the channel will be
> +          calibrated with the 0.625V and 1.25V reference channels, also
> +          known as an absolute calibration.
> +
> +- qcom,ratiometric-ref:
> +  Value type: <u32>
> +  Definition: The reference voltage pair when using ratiometric
> +          calibration:
> +	  0 = XO_IN/XOADC_GND
> +	  1 = PMIC_IN/XOADC_GND
> +	  2 = PMIC_IN/BMS_CSP
> +	  3 (invalid)
> +	  4 = XOADC_GND/XOADC_GND
> +	  5 = XOADC_VREF/XOADC_GND
> +
> +Example:
> +
> +xoadc: xoadc@197 {
> +	compatible = "qcom,pm8058-adc";
> +	reg = <0x197>;
> +	interrupt-parent = <&pm8058>;
> +	interrupts = <76 1>;
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	#io-channel-cells = <1>;
> +
> +	vcoin {
> +		reg = <0x00>;

If you have a reg property, then you should have a unit address on all 
of these.

These should really all be something like adc-channel@... Not sure if we 
have a standard node name for ADC channels. Or don't put any of this in 
DT.

> +	};
> +	vbat {
> +		reg = <0x01>;
> +	};
> +	dcin {
> +		reg = <0x02>;
> +	};
> +	ichg {
> +		reg = <0x03>;
> +	};
> +	vph_pwr {
> +		reg = <0x04>;
> +	};
> +	mpp5 {
> +		reg = <0x05>;
> +	};
> +	mpp6 {
> +		reg = <0x06>;
> +	};
> +	mpp7 {
> +		reg = <0x07>;
> +	};
> +	mpp8 {
> +		reg = <0x08>;
> +	};
> +	mpp9 {
> +		reg = <0x09>;
> +	};
> +	usb_vbus {
> +		reg = <0x0a>;
> +	};
> +	die_temp {
> +		reg = <0x0b>;
> +	};
> +	ref_625mv {
> +		reg = <0x0c>;
> +	};
> +	ref_1250mv {
> +		reg = <0x0d>;
> +	};
> +	ref_325mv {
> +		reg = <0x0e>;
> +	};
> +	ref_muxoff {
> +		reg = <0x0f>;
> +	};
> +};

^ permalink raw reply

* Re: [PATCH 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
From: Rob Herring @ 2016-12-19 22:35 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo,
	mark.rutland, netdev, devicetree, linux-kernel, mgreer, justin
In-Reply-To: <1481841044-4314-2-git-send-email-glansberry@gmail.com>

On Thu, Dec 15, 2016 at 05:30:43PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <geoff@kuvee.com>
> 
> ---
>  Documentation/devicetree/bindings/net/nfc/trf7970a.txt |  2 ++
>  drivers/nfc/trf7970a.c                                 | 13 ++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 9dda879..208f045 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -21,6 +21,7 @@ Optional SoC Specific Properties:
>  - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
>    where an extra byte is returned by Read Multiple Block commands issued
>    to Type 5 tags.
> +- vdd_io_1v8: Set to specify that the trf7970a io voltage should be set to 1.8V

Use the regulator binding and provide a fixed 1.8V supply.

>  - crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
>  
>  
> @@ -45,6 +46,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>  		irq-status-read-quirk;
>  		en2-rf-quirk;
>  		t5t-rmb-extra-byte-quirk;
> +		vdd_io_1v8;
>  		crystal_27mhz;
>  		status = "okay";
>  	};

^ permalink raw reply

* Re: [PATCH 1/3] NFC: trf7970a: add device tree option for 27MHz clock
From: Rob Herring @ 2016-12-19 22:31 UTC (permalink / raw)
  To: Geoff Lansberry
  Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo,
	mark.rutland, netdev, devicetree, linux-kernel, mgreer, justin
In-Reply-To: <1481841044-4314-1-git-send-email-glansberry@gmail.com>

On Thu, Dec 15, 2016 at 05:30:42PM -0500, Geoff Lansberry wrote:
> From: Geoff Lansberry <geoff@kuvee.com>
> 
> ---
>  .../devicetree/bindings/net/nfc/trf7970a.txt       |  3 ++
>  drivers/nfc/trf7970a.c                             | 42 ++++++++++++++++------
>  2 files changed, 34 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> index 32b35a0..9dda879 100644
> --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
> @@ -21,6 +21,8 @@ Optional SoC Specific Properties:
>  - t5t-rmb-extra-byte-quirk: Specify that the trf7970a has the erratum
>    where an extra byte is returned by Read Multiple Block commands issued
>    to Type 5 tags.
> +- crystal_27mhz: Set to specify that the input frequency to the trf7970a is 27.12MHz
> +

Can't you use 'clock-frequency = "27000000";'?

>  
>  Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>  
> @@ -43,6 +45,7 @@ Example (for ARM-based BeagleBone with TRF7970A on SPI1):
>  		irq-status-read-quirk;
>  		en2-rf-quirk;
>  		t5t-rmb-extra-byte-quirk;
> +		crystal_27mhz;
>  		status = "okay";
>  	};
>  };

^ permalink raw reply

* Re: [PATCH V5 2/8] Documentation: devicetree: thermal: da9062/61 TJUNC temperature binding
From: Rob Herring @ 2016-12-19 22:28 UTC (permalink / raw)
  To: Steve Twiss
  Cc: DEVICETREE, Eduardo Valentin, LINUX-KERNEL, LINUX-PM,
	Mark Rutland, Zhang Rui, Dmitry Torokhov, Guenter Roeck,
	LINUX-INPUT, LINUX-WATCHDOG, Lee Jones, Liam Girdwood,
	Lukasz Luba, Mark Brown, Support Opensource, Wim Van Sebroeck
In-Reply-To: <ae903f7c3c6325420b90c7f691655d902d137d1f.1481828921.git.stwiss.opensource@diasemi.com>

On Thu, Dec 15, 2016 at 07:08:39PM +0000, Steve Twiss wrote:
> From: Steve Twiss <stwiss.opensource@diasemi.com>
> 
> Device tree binding information for DA9062 and DA9061 thermal junction
> temperature monitor.
> 
> Binding descriptions for the DA9061 and DA9062 thermal TJUNC supervisor
> device driver, using a single THERMAL_TRIP_HOT trip-wire and allowing for
> a configurable polling period for over-temperature polling.
> 
> This patch also adds two examples, one for DA9062 and one for DA9061. The 
> DA9061 example uses a fall-back compatible string for the DA9062.
> 
> Signed-off-by: Steve Twiss <stwiss.opensource@diasemi.com>
> 
> ---
> This patch applies against linux-next and v4.9
> 
> v4 -> v5
>  - Rebased from v4.8 to v4.9
>  - Updates from comments by Eduardo Valentin
>  - Replace vendor defined dlg,tjunc-temp-polling-period-ms with standard
>    thermal core polling-delay-passive as part of the device tree
>    initialisation
>  - Remove Acked-by Rob Herring

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH v5 2/9] doc: DT: venus: binding document for Qualcomm video driver
From: Rob Herring @ 2016-12-19 22:21 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Andy Gross, Bjorn Andersson,
	Stephen Boyd, Srinivas Kandagatla, linux-media, linux-kernel,
	linux-arm-msm, Mark Rutland, devicetree
In-Reply-To: <1481822544-29900-3-git-send-email-stanimir.varbanov@linaro.org>

On Thu, Dec 15, 2016 at 07:22:17PM +0200, Stanimir Varbanov wrote:
> Add binding document for Venus video encoder/decoder driver
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  .../devicetree/bindings/media/qcom,venus.txt       | 68 ++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/qcom,venus.txt

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH 2/2] Documentation: ehci-omap: remove the unnecessary newline
From: Rob Herring @ 2016-12-19 22:20 UTC (permalink / raw)
  To: yegorslists; +Cc: linux-kernel, devicetree, mark.rutland
In-Reply-To: <1481817370-28242-2-git-send-email-yegorslists@googlemail.com>

On Thu, Dec 15, 2016 at 04:56:10PM +0100, yegorslists@googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
> 
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
>  Documentation/devicetree/bindings/usb/ehci-omap.txt | 1 -
>  1 file changed, 1 deletion(-)

Applied, thanks.

Rob

^ permalink raw reply

* Re: [PATCH 1/2] Documentation: omap-usb-host: fix OMAP OHCI/EHCI file names
From: Rob Herring @ 2016-12-19 22:19 UTC (permalink / raw)
  To: yegorslists; +Cc: linux-kernel, devicetree, mark.rutland
In-Reply-To: <1481817370-28242-1-git-send-email-yegorslists@googlemail.com>

On Thu, Dec 15, 2016 at 04:56:09PM +0100, yegorslists@googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
> 
> OMAP related files are actually named ehci-omap.txt and ohci-omap3.txt.
> 
> Also add full path to ohci-omap3.txt.
> 
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
>  Documentation/devicetree/bindings/mfd/omap-usb-host.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

Rob

^ permalink raw reply


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