[PATCH 13/16] net: phy: adin: implement Energy Detect Powerdown mode
The ADIN PHYs support Energy Detect Powerdown mode, which puts the PHY into
a low power mode when there is no signal on the wire (typically cable
unplugged).
This behavior is enabled by default, but can be disabled via device
property.
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cf99ccacfeeb..86848444bd98 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -27,6 +27,11 @@
#define ADIN1300_AUTO_MDI_EN BIT(10)
#define ADIN1300_MAN_MDIX_EN BIT(9)
+#define ADIN1300_PHY_CTRL_STATUS2 0x0015
+#define ADIN1300_NRG_PD_EN BIT(3)
+#define ADIN1300_NRG_PD_TX_ENBIT(2)
+#define ADIN1300_NRG_PD_STATUS BIT(1)
+
#define ADIN1300_INT_MASK_REG 0x0018
#define ADIN1300_INT_MDIO_SYNC_ENBIT(9)
#define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
@@ -95,10 +100,12 @@ static struct clause22_mmd_map clause22_mmd_map[] = {
* struct adin_priv - ADIN PHY driver private data
* gpiod_reset optional reset GPIO, to be used in soft_reset() cb
* eee_modes EEE modes to advertise after reset
+ * edpd_enabledtrue if Energy Detect Powerdown mode is enabled
*/
struct adin_priv {
struct gpio_desc*gpiod_reset;
u8 eee_modes;
+ booledpd_enabled;
};
static int adin_get_phy_internal_mode(struct phy_device *phydev)
@@ -235,6 +242,18 @@ static int adin_config_init_eee(struct phy_device *phydev)
return phy_write_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_EEE_ADV_REG, reg);
}
+static int adin_config_init_edpd(struct phy_device *phydev)
+{
+ struct adin_priv *priv = phydev->priv;
+
+ if (priv->edpd_enabled)
+ return phy_set_bits(phydev, ADIN1300_PHY_CTRL_STATUS2,
+ (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN));
+
+ return phy_clear_bits(phydev, ADIN1300_PHY_CTRL_STATUS2,
+ (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN));
+}
+
static int adin_config_init(struct phy_device *phydev)
{
phy_interface_t interface, rc;
@@ -261,6 +280,10 @@ static int adin_config_init(struct phy_device *phydev)
if (rc < 0)
return rc;
+ rc = adin_config_init_edpd(phydev);
+ if (rc < 0)
+ return rc;
+
if (phydev->interface == interface)
dev_info(&phydev->mdio.dev, "PHY is using mode '%s'\n",
phy_modes(phydev->interface));
@@ -535,6 +558,10 @@ static int adin_probe(struct phy_device *phydev)
priv->gpiod_reset = gpiod_reset;
if (device_property_read_bool(dev, "adi,eee-enabled"))
priv->eee_modes = (MDIO_EEE_100TX | MDIO_EEE_1000T);
+ if (device_property_read_bool(dev, "adi,disable-energy-detect"))
+ priv->edpd_enabled = false;
+ else
+ priv->edpd_enabled = true;
phydev->priv = priv;
return adin_reset(phydev);
--
2.20.1
[PATCH 10/16] net: phy: adin: add EEE translation layer for Clause 22
The ADIN1200 & ADIN1300 PHYs support EEE by using standard Clause 45 access
to access MMD registers for EEE.
The EEE register addresses (when using Clause 22) are available at
different addresses (than Clause 45), and since accessing these regs (via
Clause 22) needs a special mechanism, a translation table is required to
convert these addresses.
For Clause 45, this is not needed; the addresses are available as specified
by IEEE.
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 61 --
1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 31c600b7ec66..3c559a3ba487 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -44,6 +44,17 @@
#define ADIN1300_PHY_STATUS1 0x001a
#define ADIN1300_PAIR_01_SWAPBIT(11)
+/* EEE register addresses, accessible via Clause 22 access using
+ * ADIN1300_MII_EXT_REG_PTR & ADIN1300_MII_EXT_REG_DATA.
+ * The bit-fields are the same as specified by IEEE, and can be
+ * accessed via standard Clause 45 access.
+ */
+#define ADIN1300_EEE_CAP_REG 0x8000
+#define ADIN1300_EEE_ADV_REG 0x8001
+#define ADIN1300_EEE_LPABLE_REG0x8002
+#define ADIN1300_CLOCK_STOP_REG0x9400
+#define ADIN1300_LPI_WAKE_ERR_CNT_REG 0xa000
+
#define ADIN1300_GE_RGMII_CFG_REG 0xff23
#define ADIN1300_GE_RGMII_RX_MSK GENMASK(8, 6)
#define ADIN1300_GE_RGMII_RX_SEL(x) \
@@ -61,6 +72,20 @@
FIELD_PREP(ADIN1300_GE_RMII_FIFO_DEPTH_MSK, x)
#define ADIN1300_GE_RMII_EN BIT(0)
+struct clause22_mmd_map {
+ int devad;
+ u16 cl22_regnum;
+ u16 adin_regnum;
+};
+
+static struct clause22_mmd_map clause22_mmd_map[] = {
+ { MDIO_MMD_PCS, MDIO_PCS_EEE_ABLE, ADIN1300_EEE_CAP_REG },
+ { MDIO_MMD_AN, MDIO_AN_EEE_LPABLE, ADIN1300_EEE_LPABLE_REG },
+ { MDIO_MMD_AN, MDIO_AN_EEE_ADV,ADIN1300_EEE_ADV_REG },
+ { MDIO_MMD_PCS, MDIO_CTRL1, ADIN1300_CLOCK_STOP_REG },
+ { MDIO_MMD_PCS, MDIO_PCS_EEE_WK_ERR,ADIN1300_LPI_WAKE_ERR_CNT_REG },
+};
+
static int adin_get_phy_internal_mode(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
@@ -233,10 +258,31 @@ static int adin_phy_config_intr(struct phy_device *phydev)
ADIN1300_INT_MASK_EN);
}
+static int adin_cl22_to_adin_reg(int devad, u16 cl22_regnum)
+{
+ struct clause22_mmd_map *m;
+ int i;
+
+ if (devad == MDIO_MMD_VEND1)
+ return cl22_regnum;
+
+ for (i = 0; i < ARRAY_SIZE(clause22_mmd_map); i++) {
+ m = &clause22_mmd_map[i];
+ if (m->devad == devad && m->cl22_regnum == cl22_regnum)
+ return m->adin_regnum;
+ }
+
+ pr_err("No translation available for devad: %d reg: %04x\n",
+ devad, cl22_regnum);
+
+ return -EINVAL;
+}
+
static int adin_read_mmd(struct phy_device *phydev, int devad, u16 regnum)
{
struct mii_bus *bus = phydev->mdio.bus;
int phy_addr = phydev->mdio.addr;
+ int adin_regnum;
int err;
if (phydev->is_c45) {
@@ -245,7 +291,12 @@ static int adin_read_mmd(struct phy_device *phydev, int
devad, u16 regnum)
return __mdiobus_read(bus, phy_addr, addr);
}
- err = __mdiobus_write(bus, phy_addr, ADIN1300_MII_EXT_REG_PTR, regnum);
+ adin_regnum = adin_cl22_to_adin_reg(devad, regnum);
+ if (adin_regnum < 0)
+ return adin_regnum;
+
+ err = __mdiobus_write(bus, phy_addr, ADIN1300_MII_EXT_REG_PTR,
+ adin_regnum);
if (err)
return err;
@@ -257,6 +308,7 @@ static int adin_write_mmd(struct phy_device *phydev, int
devad, u16 regnum,
{
struct mii_bus *bus = phydev->mdio.bus;
int phy_addr = phydev->mdio.addr;
+ int adin_regnum;
int err;
if (phydev->is_c45) {
@@ -265,7 +317,12 @@ static int adin_write_mmd(struct phy_device *phydev, int
devad, u16 regnum,
return __mdiobus_write(bus, phy_addr, addr, val);
}
- err = __mdiobus_write(bus, phy_addr, ADIN1300_MII_EXT_REG_PTR, regnum);
+ adin_regnum = adin_cl22_to_adin_reg(devad, regnum);
+ if (adin_regnum < 0)
+ return adin_regnum;
+
+ err = __mdiobus_write(bus, phy_addr, ADIN1300_MII_EXT_REG_PTR,
+ adin_regnum);
if (err)
return err;
--
2.20.1
[PATCH 12/16] net: phy: adin: read EEE setting from device-tree
By default, EEE is not advertised on system init. This change allows the
user to specify a device property to enable EEE advertisements when the PHY
initializes.
Also, before resetting the PHY, the EEE settings are read, so that after
the reset is complete, they are written back into the EEE advertisement
register.
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 476a81ce9341..cf99ccacfeeb 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -94,9 +94,11 @@ static struct clause22_mmd_map clause22_mmd_map[] = {
/**
* struct adin_priv - ADIN PHY driver private data
* gpiod_reset optional reset GPIO, to be used in soft_reset() cb
+ * eee_modes EEE modes to advertise after reset
*/
struct adin_priv {
struct gpio_desc*gpiod_reset;
+ u8 eee_modes;
};
static int adin_get_phy_internal_mode(struct phy_device *phydev)
@@ -216,6 +218,23 @@ static int adin_config_rmii_mode(struct phy_device *phydev,
ADIN1300_GE_RMII_CFG_REG, reg);
}
+static int adin_config_init_eee(struct phy_device *phydev)
+{
+ struct adin_priv *priv = phydev->priv;
+ int reg;
+
+ reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_EEE_ADV_REG);
+ if (reg < 0)
+ return reg;
+
+ if (priv->eee_modes)
+ reg |= priv->eee_modes;
+ else
+ reg &= ~(MDIO_EEE_100TX | MDIO_EEE_1000T);
+
+ return phy_write_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_EEE_ADV_REG, reg);
+}
+
static int adin_config_init(struct phy_device *phydev)
{
phy_interface_t interface, rc;
@@ -238,6 +257,10 @@ static int adin_config_init(struct phy_device *phydev)
if (rc < 0)
return rc;
+ rc = adin_config_init_eee(phydev);
+ if (rc < 0)
+ return rc;
+
if (phydev->interface == interface)
dev_info(&phydev->mdio.dev, "PHY is using mode '%s'\n",
phy_modes(phydev->interface));
@@ -473,6 +496,12 @@ static int adin_reset(struct phy_device *phydev)
struct adin_priv *priv = phydev->priv;
int ret;
+ /* Update EEE settings before resetting, in case ethtool changed them */
+ ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_EEE_ADV_REG);
+ if (ret < 0)
+ return ret;
+ priv->eee_modes = (ret & (MDIO_EEE_100TX | MDIO_EEE_1000T));
+
if (priv->gpiod_reset) {
/* GPIO reset requires min 10 uS low,
* 5 msecs max before we know that the interface is up again
@@ -504,6 +533,8 @@ static int adin_probe(struct phy_device *phydev)
gpiod_reset = NULL;
priv->gpiod_reset = gpiod_reset;
+ if (device_property_read_bool(dev, "adi,eee-enabled"))
+ priv->eee_modes = (MDIO_EEE_100TX | MDIO_EEE_1000T);
phydev->priv = priv;
return adin_reset(phydev);
--
2.20.1
[PATCH 15/16] net: phy: adin: add ethtool get_stats support
This change implements retrieving all the error counters from the PHY.
The PHY supports several error counters/stats. The `Mean Square Errors`
status values are only valie when a link is established, and shouldn't be
incremented. These values characterize the quality of a signal.
The rest of the error counters are self-clearing on read.
Most of them are reports from the Frame Checker engine that the PHY has.
Not retrieving the `LPI Wake Error Count Register` here, since that is used
by the PHY framework to check for any EEE errors. And that register is
self-clearing when read (as per IEEE spec).
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 108 +
1 file changed, 108 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index a1f3456a8504..04896547dac8 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -103,6 +103,32 @@ static struct clause22_mmd_map clause22_mmd_map[] = {
{ MDIO_MMD_PCS, MDIO_PCS_EEE_WK_ERR,ADIN1300_LPI_WAKE_ERR_CNT_REG },
};
+struct adin_hw_stat {
+ const char *string;
+ u16 reg1;
+ u16 reg2;
+ bool do_not_inc;
+};
+
+/* Named just like in the datasheet */
+static struct adin_hw_stat adin_hw_stats[] = {
+ { "RxErrCnt", 0x0014, },
+ { "MseA", 0x8402, 0, true },
+ { "MseB", 0x8403, 0, true },
+ { "MseC", 0x8404, 0, true },
+ { "MseD", 0x8405, 0, true },
+ { "FcFrmCnt", 0x940A, 0x940B }, /* FcFrmCntH + FcFrmCntL */
+ { "FcLenErrCnt",0x940C },
+ { "FcAlgnErrCnt", 0x940D },
+ { "FcSymbErrCnt", 0x940E },
+ { "FcOszCnt", 0x940F },
+ { "FcUszCnt", 0x9410 },
+ { "FcOddCnt", 0x9411 },
+ { "FcOddPreCnt",0x9412 },
+ { "FcDribbleBitsCnt", 0x9413 },
+ { "FcFalseCarrierCnt", 0x9414 },
+};
+
/**
* struct adin_priv - ADIN PHY driver private data
* gpiod_reset optional reset GPIO, to be used in soft_reset() cb
@@ -113,6 +139,7 @@ struct adin_priv {
struct gpio_desc*gpiod_reset;
u8 eee_modes;
booledpd_enabled;
+ u64 stats[ARRAY_SIZE(adin_hw_stats)];
};
static int adin_get_phy_internal_mode(struct phy_device *phydev)
@@ -568,6 +595,81 @@ static int adin_reset(struct phy_device *phydev)
return adin_subsytem_soft_reset(phydev);
}
+static int adin_get_sset_count(struct phy_device *phydev)
+{
+ return ARRAY_SIZE(adin_hw_stats);
+}
+
+static void adin_get_strings(struct phy_device *phydev, u8 *data)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(adin_hw_stats); i++) {
+ memcpy(data + i * ETH_GSTRING_LEN,
+ adin_hw_stats[i].string, ETH_GSTRING_LEN);
+ }
+}
+
+static int adin_read_mmd_stat_regs(struct phy_device *phydev,
+ struct adin_hw_stat *stat,
+ u32 *val)
+{
+ int ret;
+
+ ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->reg1);
+ if (ret < 0)
+ return ret;
+
+ *val = (ret & 0x);
+
+ if (stat->reg2 == 0)
+ return 0;
+
+ ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, stat->reg2);
+ if (ret < 0)
+ return ret;
+
+ *val <<= 16;
+ *val |= (ret & 0x);
+
+ return 0;
+}
+
+static u64 adin_get_stat(struct phy_device *phydev, int i)
+{
+ struct adin_hw_stat *stat = &adin_hw_stats[i];
+ struct adin_priv *priv = phydev->priv;
+ u32 val;
+ int ret;
+
+ if (stat->reg1 > 0x1f) {
+ ret = adin_read_mmd_stat_regs(phydev, stat, &val);
+ if (ret < 0)
+ return (u64)(~0);
+ } else {
+ ret = phy_read(phydev, stat->reg1);
+ if (ret < 0)
+ return (u64)(~0);
+ val = (ret & 0x);
+ }
+
+ if (stat->do_not_inc)
+ priv->stats[i] = val;
+ else
+ priv->stats[i] += val;
+
+ return priv->stats[i];
+}
+
+static void adin_get_stats(struct phy_device *phydev,
+ struct ethtool_stats *stats, u64 *data)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(adin_hw_stats); i++)
+ data[i] = adin_get_stat(phydev, i);
+}
+
static int adin_probe(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
@@ -607,6 +709,9 @@ static struct phy_driver adin_driver[] = {
.read_status= adin_read_status,
[PATCH 11/16] net: phy: adin: PHY reset mechanisms
The ADIN PHYs supports 4 types of reset:
1. The standard PHY reset via BMCR_RESET bit in MII_BMCR reg
2. Reset via GPIO
3. Reset via reg GeSftRst (0xff0c) & reload previous pin configs
4. Reset via reg GeSftRst (0xff0c) & request new pin configs
Resets 2 & 4 are almost identical, with the exception that the crystal
oscillator is available during reset for 2.
Resetting via GeSftRst or via GPIO is useful when doing a warm reboot. If
doing various settings via phytool or ethtool, the sub-system registers
don't reset just via BMCR_RESET.
This change implements resetting the entire PHY subsystem during probe.
During PHY HW init (phy_hw_init() logic) the PHY core regs will be reset
again via BMCR_RESET. This will also need to happen during a PM resume.
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 82 ++
1 file changed, 82 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 3c559a3ba487..476a81ce9341 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -6,12 +6,14 @@
*/
#include
#include
+#include
#include
#include
#include
#include
#include
#include
+#include
#include
@@ -55,6 +57,9 @@
#define ADIN1300_CLOCK_STOP_REG0x9400
#define ADIN1300_LPI_WAKE_ERR_CNT_REG 0xa000
+#define ADIN1300_GE_SOFT_RESET_REG 0xff0c
+#define ADIN1300_GE_SOFT_RESET BIT(0)
+
#define ADIN1300_GE_RGMII_CFG_REG 0xff23
#define ADIN1300_GE_RGMII_RX_MSK GENMASK(8, 6)
#define ADIN1300_GE_RGMII_RX_SEL(x) \
@@ -86,6 +91,14 @@ static struct clause22_mmd_map clause22_mmd_map[] = {
{ MDIO_MMD_PCS, MDIO_PCS_EEE_WK_ERR,ADIN1300_LPI_WAKE_ERR_CNT_REG },
};
+/**
+ * struct adin_priv - ADIN PHY driver private data
+ * gpiod_reset optional reset GPIO, to be used in soft_reset() cb
+ */
+struct adin_priv {
+ struct gpio_desc*gpiod_reset;
+};
+
static int adin_get_phy_internal_mode(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
@@ -429,6 +442,73 @@ static int adin_read_status(struct phy_device *phydev)
return genphy_read_status(phydev);
}
+static int adin_subsytem_soft_reset(struct phy_device *phydev)
+{
+ int reg, rc, i;
+
+ reg = phy_read_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_SOFT_RESET_REG);
+ if (reg < 0)
+ return reg;
+
+ reg |= ADIN1300_GE_SOFT_RESET;
+ rc = phy_write_mmd(phydev, MDIO_MMD_VEND1, ADIN1300_GE_SOFT_RESET_REG,
+ reg);
+ if (rc < 0)
+ return rc;
+
+ for (i = 0; i < 20; i++) {
+ usleep_range(500, 1000);
+ reg = phy_read_mmd(phydev, MDIO_MMD_VEND1,
+ ADIN1300_GE_SOFT_RESET_REG);
+ if (reg < 0 || (reg & ADIN1300_GE_SOFT_RESET))
+ continue;
+ return 0;
+ }
+
+ return -ETIMEDOUT;
+}
+
+static int adin_reset(struct phy_device *phydev)
+{
+ struct adin_priv *priv = phydev->priv;
+ int ret;
+
+ if (priv->gpiod_reset) {
+ /* GPIO reset requires min 10 uS low,
+* 5 msecs max before we know that the interface is up again
+*/
+ gpiod_set_value(priv->gpiod_reset, 0);
+ usleep_range(10, 15);
+ gpiod_set_value(priv->gpiod_reset, 1);
+ mdelay(5);
+
+ return 0;
+ }
+
+ /* Reset PHY core regs & subsystem regs */
+ return adin_subsytem_soft_reset(phydev);
+}
+
+static int adin_probe(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ struct gpio_desc *gpiod_reset;
+ struct adin_priv *priv;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(gpiod_reset))
+ gpiod_reset = NULL;
+
+ priv->gpiod_reset = gpiod_reset;
+ phydev->priv = priv;
+
+ return adin_reset(phydev);
+}
+
static struct phy_driver adin_driver[] = {
{
.phy_id = PHY_ID_ADIN1200,
@@ -437,6 +517,7 @@ static struct phy_driver adin_driver[] = {
.features = PHY_BASIC_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.config_init= adin_config_init,
+ .probe = adin_probe,
.config_aneg= adin_config_aneg,
.read_status= adin_read_status,
.ack_interrupt = adin_phy_ack_intr,
@@ -453,6 +534,7 @@ static struct phy_driver adin_driver[] = {
.features = PHY_GBIT_FEATURES,
.flags = PHY_HAS_INTERRUPT,
.
[PATCH 03/16] net: phy: adin: add support for interrupts
This change adds support for enabling PHY interrupts that can be used by
the PHY framework to get signal for link/speed/auto-negotiation changes.
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 44 ++
1 file changed, 44 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index c100a0dd95cd..b75c723bda79 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -14,6 +14,22 @@
#define PHY_ID_ADIN12000x0283bc20
#define PHY_ID_ADIN13000x0283bc30
+#define ADIN1300_INT_MASK_REG 0x0018
+#define ADIN1300_INT_MDIO_SYNC_ENBIT(9)
+#define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
+#define ADIN1300_INT_ANEG_PAGE_RX_EN BIT(6)
+#define ADIN1300_INT_IDLE_ERR_CNT_EN BIT(5)
+#define ADIN1300_INT_MAC_FIFO_OU_EN BIT(4)
+#define ADIN1300_INT_RX_STAT_CHNG_EN BIT(3)
+#define ADIN1300_INT_LINK_STAT_CHNG_EN BIT(2)
+#define ADIN1300_INT_SPEED_CHNG_EN BIT(1)
+#define ADIN1300_INT_HW_IRQ_EN BIT(0)
+#define ADIN1300_INT_MASK_EN \
+ (ADIN1300_INT_ANEG_STAT_CHNG_EN | ADIN1300_INT_ANEG_PAGE_RX_EN | \
+ADIN1300_INT_LINK_STAT_CHNG_EN | ADIN1300_INT_SPEED_CHNG_EN | \
+ADIN1300_INT_HW_IRQ_EN)
+#define ADIN1300_INT_STATUS_REG0x0019
+
static int adin_config_init(struct phy_device *phydev)
{
int rc;
@@ -25,15 +41,40 @@ static int adin_config_init(struct phy_device *phydev)
return 0;
}
+static int adin_phy_ack_intr(struct phy_device *phydev)
+{
+ int ret;
+
+ /* Clear pending interrupts. */
+ ret = phy_read(phydev, ADIN1300_INT_STATUS_REG);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int adin_phy_config_intr(struct phy_device *phydev)
+{
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ return phy_set_bits(phydev, ADIN1300_INT_MASK_REG,
+ ADIN1300_INT_MASK_EN);
+
+ return phy_clear_bits(phydev, ADIN1300_INT_MASK_REG,
+ ADIN1300_INT_MASK_EN);
+}
+
static struct phy_driver adin_driver[] = {
{
.phy_id = PHY_ID_ADIN1200,
.name = "ADIN1200",
.phy_id_mask= 0xfff0,
.features = PHY_BASIC_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
.config_init= adin_config_init,
.config_aneg= genphy_config_aneg,
.read_status= genphy_read_status,
+ .ack_interrupt = adin_phy_ack_intr,
+ .config_intr= adin_phy_config_intr,
.resume = genphy_resume,
.suspend= genphy_suspend,
},
@@ -42,9 +83,12 @@ static struct phy_driver adin_driver[] = {
.name = "ADIN1300",
.phy_id_mask= 0xfff0,
.features = PHY_GBIT_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
.config_init= adin_config_init,
.config_aneg= genphy_config_aneg,
.read_status= genphy_read_status,
+ .ack_interrupt = adin_phy_ack_intr,
+ .config_intr= adin_phy_config_intr,
.resume = genphy_resume,
.suspend= genphy_suspend,
},
--
2.20.1
[PATCH 14/16] net: phy: adin: make sure down-speed auto-neg is enabled
Down-speed auto-negotiation may not always be enabled, in which case the
PHY won't down-shift to 100 or 10 during auto-negotiation.
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 86848444bd98..a1f3456a8504 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -32,6 +32,13 @@
#define ADIN1300_NRG_PD_TX_ENBIT(2)
#define ADIN1300_NRG_PD_STATUS BIT(1)
+#define ADIN1300_PHY_CTRL2 0x0016
+#define ADIN1300_DOWNSPEED_AN_100_EN BIT(11)
+#define ADIN1300_DOWNSPEED_AN_10_EN BIT(10)
+#define ADIN1300_GROUP_MDIO_EN BIT(6)
+#define ADIN1300_DOWNSPEEDS_EN \
+ (ADIN1300_DOWNSPEED_AN_100_EN | ADIN1300_DOWNSPEED_AN_10_EN)
+
#define ADIN1300_INT_MASK_REG 0x0018
#define ADIN1300_INT_MDIO_SYNC_ENBIT(9)
#define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
@@ -425,6 +432,22 @@ static int adin_config_mdix(struct phy_device *phydev)
return phy_write(phydev, ADIN1300_PHY_CTRL1, reg);
}
+static int adin_config_downspeeds(struct phy_device *phydev)
+{
+ int reg;
+
+ reg = phy_read(phydev, ADIN1300_PHY_CTRL2);
+ if (reg < 0)
+ return reg;
+
+ if ((reg & ADIN1300_DOWNSPEEDS_EN) == ADIN1300_DOWNSPEEDS_EN)
+ return 0;
+
+ reg |= ADIN1300_DOWNSPEEDS_EN;
+
+ return phy_write(phydev, ADIN1300_PHY_CTRL2, reg);
+}
+
static int adin_config_aneg(struct phy_device *phydev)
{
int ret;
@@ -433,6 +456,10 @@ static int adin_config_aneg(struct phy_device *phydev)
if (ret)
return ret;
+ ret = adin_config_downspeeds(phydev);
+ if (ret < 0)
+ return ret;
+
return genphy_config_aneg(phydev);
}
--
2.20.1
[PATCH 07/16] net: phy: adin: make RGMII internal delays configurable
The internal delays for the RGMII are configurable for both RX & TX. This
change adds support for configuring them via device-tree (or ACPI).
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index e3d2ff8cc09c..cb96d47d457e 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -5,6 +5,7 @@
* Copyright 2019 Analog Devices Inc.
*/
#include
+#include
#include
#include
#include
@@ -12,6 +13,8 @@
#include
#include
+#include
+
#define PHY_ID_ADIN12000x0283bc20
#define PHY_ID_ADIN13000x0283bc30
@@ -35,6 +38,12 @@
#define ADIN1300_INT_STATUS_REG0x0019
#define ADIN1300_GE_RGMII_CFG_REG 0xff23
+#define ADIN1300_GE_RGMII_RX_MSK GENMASK(8, 6)
+#define ADIN1300_GE_RGMII_RX_SEL(x) \
+ FIELD_PREP(ADIN1300_GE_RGMII_RX_MSK, x)
+#define ADIN1300_GE_RGMII_GTX_MSKGENMASK(5, 3)
+#define ADIN1300_GE_RGMII_GTX_SEL(x) \
+ FIELD_PREP(ADIN1300_GE_RGMII_GTX_MSK, x)
#define ADIN1300_GE_RGMII_RXID_ENBIT(2)
#define ADIN1300_GE_RGMII_TXID_ENBIT(1)
#define ADIN1300_GE_RGMII_EN BIT(0)
@@ -67,6 +76,32 @@ static int adin_get_phy_internal_mode(struct phy_device
*phydev)
return -EINVAL;
}
+static void adin_config_rgmii_rx_internal_delay(struct phy_device *phydev,
+ int *reg)
+{
+ struct device *dev = &phydev->mdio.dev;
+ u32 val;
+
+ if (device_property_read_u32(dev, "adi,rx-internal-delay", &val))
+ val = ADIN1300_RGMII_2_00_NS;
+
+ *reg &= ADIN1300_GE_RGMII_RX_MSK;
+ *reg |= ADIN1300_GE_RGMII_RX_SEL(val);
+}
+
+static void adin_config_rgmii_tx_internal_delay(struct phy_device *phydev,
+ int *reg)
+{
+ struct device *dev = &phydev->mdio.dev;
+ u32 val;
+
+ if (device_property_read_u32(dev, "adi,tx-internal-delay", &val))
+ val = ADIN1300_RGMII_2_00_NS;
+
+ *reg &= ADIN1300_GE_RGMII_GTX_MSK;
+ *reg |= ADIN1300_GE_RGMII_GTX_SEL(val);
+}
+
static int adin_config_rgmii_mode(struct phy_device *phydev,
phy_interface_t intf)
{
@@ -86,6 +121,7 @@ static int adin_config_rgmii_mode(struct phy_device *phydev,
if (intf == PHY_INTERFACE_MODE_RGMII_ID ||
intf == PHY_INTERFACE_MODE_RGMII_RXID) {
reg |= ADIN1300_GE_RGMII_RXID_EN;
+ adin_config_rgmii_rx_internal_delay(phydev, ®);
} else {
reg &= ~ADIN1300_GE_RGMII_RXID_EN;
}
@@ -93,6 +129,7 @@ static int adin_config_rgmii_mode(struct phy_device *phydev,
if (intf == PHY_INTERFACE_MODE_RGMII_ID ||
intf == PHY_INTERFACE_MODE_RGMII_TXID) {
reg |= ADIN1300_GE_RGMII_TXID_EN;
+ adin_config_rgmii_tx_internal_delay(phydev, ®);
} else {
reg &= ~ADIN1300_GE_RGMII_TXID_EN;
}
--
2.20.1
[PATCH 01/16] net: phy: adin: add support for Analog Devices PHYs
This change adds support for Analog Devices Industrial Ethernet PHYs.
Particularly the PHYs this driver adds support for:
* ADIN1200 - Robust, Industrial, Low Power 10/100 Ethernet PHY
* ADIN1300 - Robust, Industrial, Low Latency 10/100/1000 Gigabit
Ethernet PHY
The 2 chips are pin & register compatible with one another. The main
difference being that ADIN1200 doesn't operate in gigabit mode.
The chips can be operated by the Generic PHY driver as well via the
standard IEEE PHY registers (0x - 0x000F) which are supported by the
kernel as well. This assumes that configuration of the PHY has been done
required.
Configuration can also be done via registers, which will be implemented by
the driver in the next changes.
Datasheets:
https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1300.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1200.pdf
Signed-off-by: Alexandru Ardelean
---
MAINTAINERS | 7 +
drivers/net/phy/Kconfig | 9 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/adin.c | 59
4 files changed, 76 insertions(+)
create mode 100644 drivers/net/phy/adin.c
diff --git a/MAINTAINERS b/MAINTAINERS
index ee663e0e2f2e..faf5723610c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -938,6 +938,13 @@ S: Supported
F: drivers/mux/adgs1408.c
F: Documentation/devicetree/bindings/mux/adi,adgs1408.txt
+ANALOG DEVICES INC ADIN DRIVER
+M: Alexandru Ardelean
+L: net...@vger.kernel.org
+W: http://ez.analog.com/community/linux-device-drivers
+S: Supported
+F: drivers/net/phy/adin.c
+
ANALOG DEVICES INC ADIS DRIVER LIBRARY
M: Alexandru Ardelean
S: Supported
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 206d8650ee7f..5966d3413676 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -257,6 +257,15 @@ config SFP
depends on HWMON || HWMON=n
select MDIO_I2C
+config ADIN_PHY
+ tristate "Analog Devices Industrial Ethernet PHYs"
+ help
+ Adds support for the Analog Devices Industrial Ethernet PHYs.
+ Currently supports the:
+ - ADIN1200 - Robust,Industrial, Low Power 10/100 Ethernet PHY
+ - ADIN1300 - Robust,Industrial, Low Latency 10/100/1000 Gigabit
+ Ethernet PHY
+
config AMD_PHY
tristate "AMD PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index ba07c27e4208..a03437e091f3 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_SFP) += sfp.o
sfp-obj-$(CONFIG_SFP) += sfp-bus.o
obj-y += $(sfp-obj-y) $(sfp-obj-m)
+obj-$(CONFIG_ADIN_PHY) += adin.o
obj-$(CONFIG_AMD_PHY) += amd.o
aquantia-objs += aquantia_main.o
ifdef CONFIG_HWMON
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
new file mode 100644
index ..6a610d4563c3
--- /dev/null
+++ b/drivers/net/phy/adin.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0+
+/**
+ * Driver for Analog Devices Industrial Ethernet PHYs
+ *
+ * Copyright 2019 Analog Devices Inc.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define PHY_ID_ADIN12000x0283bc20
+#define PHY_ID_ADIN13000x0283bc30
+
+static int adin_config_init(struct phy_device *phydev)
+{
+ int rc;
+
+ rc = genphy_config_init(phydev);
+ if (rc < 0)
+ return rc;
+
+ return 0;
+}
+
+static struct phy_driver adin_driver[] = {
+ {
+ .phy_id = PHY_ID_ADIN1200,
+ .name = "ADIN1200",
+ .phy_id_mask= 0xfff0,
+ .features = PHY_BASIC_FEATURES,
+ .config_init= adin_config_init,
+ .config_aneg= genphy_config_aneg,
+ .read_status= genphy_read_status,
+ },
+ {
+ .phy_id = PHY_ID_ADIN1300,
+ .name = "ADIN1300",
+ .phy_id_mask= 0xfff0,
+ .features = PHY_GBIT_FEATURES,
+ .config_init= adin_config_init,
+ .config_aneg= genphy_config_aneg,
+ .read_status= genphy_read_status,
+ },
+};
+
+module_phy_driver(adin_driver);
+
+static struct mdio_device_id __maybe_unused adin_tbl[] = {
+ { PHY_ID_ADIN1200, 0xfff0 },
+ { PHY_ID_ADIN1300, 0xfff0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(mdio, adin_tbl);
+MODULE_DESCRIPTION("Analog Devices Industrial Ethernet PHY driver");
+MODULE_LICENSE("GPL");
--
2.20.1
[PATCH 16/16] dt-bindings: net: add bindings for ADIN PHY driver
This change adds bindings for the Analog Devices ADIN PHY driver, detailing
all the properties implemented by the driver.
Signed-off-by: Alexandru Ardelean
---
.../devicetree/bindings/net/adi,adin.yaml | 93 +++
MAINTAINERS | 2 +
include/dt-bindings/net/adin.h| 26 ++
3 files changed, 121 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml
create mode 100644 include/dt-bindings/net/adin.h
diff --git a/Documentation/devicetree/bindings/net/adi,adin.yaml
b/Documentation/devicetree/bindings/net/adi,adin.yaml
new file mode 100644
index ..fcf884bb86f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/adi,adin.yaml
@@ -0,0 +1,93 @@
+# SPDX-License-Identifier: GPL-2.0+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/adi,adin.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIN1200/ADIN1300 PHY
+
+maintainers:
+ - Alexandru Ardelean
+
+description: |
+ Bindings for Analog Devices Industrial Ethernet PHYs
+
+properties:
+ compatible:
+description: |
+ Compatible list, may contain "ethernet-phy-ieee802.3-c45" in which case
+ Clause 45 will be used to access device management registers. If
+ unspecified, Clause 22 will be used. Use this only when MDIO supports
+ Clause 45 access, but there is no other way to determine this.
+enum:
+ - ethernet-phy-ieee802.3-c45
+
+ adi,phy-mode-internal:
+$ref: /schemas/types.yaml#/definitions/string
+description: |
+ The internal mode of the PHY. This assumes that there is a PHY converter
+ in-between the MAC & PHY.
+enum: [ "rgmii", "rgmii-id", "rgmii-txid", "rgmii-rxid", "rmii", "mii" ]
+
+ adi,rx-internal-delay:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: |
+ RGMII RX Clock Delay used only when PHY operates in RGMII mode (phy-mode
+ is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
+ default value is 0 (which represents 2 ns)
+enum: [ 0, 1, 2, 6, 7 ]
+
+ adi,tx-internal-delay:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: |
+ RGMII TX Clock Delay used only when PHY operates in RGMII mode (phy-mode
+ is "rgmii-id", "rgmii-rxid", "rgmii-txid") see `dt-bindings/net/adin.h`
+ default value is 0 (which represents 2 ns)
+enum: [ 0, 1, 2, 6, 7 ]
+
+ adi,fifo-depth:
+$ref: /schemas/types.yaml#/definitions/uint32
+description: |
+ When operating in RMII mode, this option configures the FIFO depth.
+ See `dt-bindings/net/adin.h`.
+enum: [ 0, 1, 2, 3, 4, 5 ]
+
+ adi,eee-enabled:
+description: |
+ Advertise EEE capabilities on power-up/init (default disabled)
+type: boolean
+
+ adi,disable-energy-detect:
+description: |
+ Disables Energy Detect Powerdown Mode (default disabled, i.e energy
detect
+ is enabled if this property is unspecified)
+type: boolean
+
+ reset-gpios:
+description: |
+ GPIO to reset the PHY
+ see Documentation/devicetree/bindings/gpio/gpio.txt.
+
+examples:
+ - |
+ethernet-phy@0 {
+compatible = "ethernet-phy-ieee802.3-c45";
+reg = <0>;
+};
+ - |
+#include
+ethernet-phy@1 {
+reg = <1>;
+adi,phy-mode-internal = "rgmii-id";
+
+adi,rx-internal-delay = ;
+adi,tx-internal-delay = ;
+};
+ - |
+#include
+ethernet-phy@2 {
+reg = <2>;
+phy-mode = "rmii";
+
+adi,fifo-depth = ;
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index faf5723610c8..6ffbb266dee4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -944,6 +944,8 @@ L: net...@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
S: Supported
F: drivers/net/phy/adin.c
+F: include/dt-bindings/net/adin.h
+F: Documentation/devicetree/bindings/net/adi,adin.yaml
ANALOG DEVICES INC ADIS DRIVER LIBRARY
M: Alexandru Ardelean
diff --git a/include/dt-bindings/net/adin.h b/include/dt-bindings/net/adin.h
new file mode 100644
index ..4c3afa550c59
--- /dev/null
+++ b/include/dt-bindings/net/adin.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/**
+ * Device Tree constants for Analog Devices Industrial Ethernet PHYs
+ *
+ * Copyright 2019 Analog Devices Inc.
+ */
+
+#ifndef _DT_BINDINGS_ADIN_H
+#define _DT_BINDINGS_ADIN_H
+
+/* RGMII internal delay settings for rx and tx for ADIN1300 */
+#define ADIN1300_RGMII_1_60_NS 0x1
+#define ADIN1300_RGMII_1_80_NS 0x2
+#defineADIN1300_RGMII_2_00_NS 0x0
+#defineADIN1300_RGMII_2_20_NS 0x6
+#defineADIN1300_RGMII_2_40_NS 0x7
+
+/* RMII fifo depth values */
+#
[PATCH 09/16] net: phy: adin: add support MDI/MDIX/Auto-MDI selection
The ADIN PHYs support automatic MDI/MDIX negotiation. By default this is
disabled, so this is enabled at `config_init`.
This is controlled via the PHY Control 1 register.
The supported modes are:
1. Manual MDI
2. Manual MDIX
3. Auto MDIX - prefer MDIX
4. Auto MDIX - prefer MDI
The phydev mdix & mdix_ctrl fields include modes 3 & 4 into a single
auto-mode. So, the default mode this driver enables is 4 when Auto-MDI mode
is used.
When detecting MDI/MDIX mode, a combination of the PHY Control 1 register
and PHY Status 1 register is used to determine the correct MDI/MDIX mode.
If Auto-MDI mode is not set, then the manual MDI/MDIX mode is returned.
If Auto-MDI mode is set, then MDIX mode is returned differs from the
preferred MDI/MDIX mode.
This covers all cases where:
1. MDI preferred & Pair01Swapped == MDIX
2. MDIX preferred & Pair01Swapped == MDI
3. MDI preferred & ! Pair01Swapped == MDIX
4. MDIX preferred & ! Pair01Swapped == MDI
The preferred MDI/MDIX mode is not configured via SW, but can be configured
via HW pins. Note that the `Pair01Swapped` is the Green-Yellow physical
pairs.
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 117 +++--
1 file changed, 113 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 2e27ffd403b4..31c600b7ec66 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -21,6 +21,10 @@
#define ADIN1300_MII_EXT_REG_PTR 0x10
#define ADIN1300_MII_EXT_REG_DATA 0x11
+#define ADIN1300_PHY_CTRL1 0x0012
+#define ADIN1300_AUTO_MDI_EN BIT(10)
+#define ADIN1300_MAN_MDIX_EN BIT(9)
+
#define ADIN1300_INT_MASK_REG 0x0018
#define ADIN1300_INT_MDIO_SYNC_ENBIT(9)
#define ADIN1300_INT_ANEG_STAT_CHNG_EN BIT(8)
@@ -37,6 +41,9 @@
ADIN1300_INT_HW_IRQ_EN)
#define ADIN1300_INT_STATUS_REG0x0019
+#define ADIN1300_PHY_STATUS1 0x001a
+#define ADIN1300_PAIR_01_SWAPBIT(11)
+
#define ADIN1300_GE_RGMII_CFG_REG 0xff23
#define ADIN1300_GE_RGMII_RX_MSK GENMASK(8, 6)
#define ADIN1300_GE_RGMII_RX_SEL(x) \
@@ -175,6 +182,8 @@ static int adin_config_init(struct phy_device *phydev)
{
phy_interface_t interface, rc;
+ phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
+
rc = genphy_config_init(phydev);
if (rc < 0)
return rc;
@@ -263,6 +272,106 @@ static int adin_write_mmd(struct phy_device *phydev, int
devad, u16 regnum,
return __mdiobus_write(bus, phy_addr, ADIN1300_MII_EXT_REG_DATA, val);
}
+static int adin_config_mdix(struct phy_device *phydev)
+{
+ bool auto_en, mdix_en;
+ int reg;
+
+ mdix_en = false;
+ auto_en = false;
+ switch (phydev->mdix_ctrl) {
+ case ETH_TP_MDI:
+ break;
+ case ETH_TP_MDI_X:
+ mdix_en = true;
+ break;
+ case ETH_TP_MDI_AUTO:
+ auto_en = true;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ reg = phy_read(phydev, ADIN1300_PHY_CTRL1);
+ if (reg < 0)
+ return reg;
+
+ if (mdix_en)
+ reg |= ADIN1300_MAN_MDIX_EN;
+ else
+ reg &= ~ADIN1300_MAN_MDIX_EN;
+
+ if (auto_en)
+ reg |= ADIN1300_AUTO_MDI_EN;
+ else
+ reg &= ~ADIN1300_AUTO_MDI_EN;
+
+ return phy_write(phydev, ADIN1300_PHY_CTRL1, reg);
+}
+
+static int adin_config_aneg(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = adin_config_mdix(phydev);
+ if (ret)
+ return ret;
+
+ return genphy_config_aneg(phydev);
+}
+
+static int adin_mdix_update(struct phy_device *phydev)
+{
+ bool auto_en, mdix_en;
+ bool swapped;
+ int reg;
+
+ reg = phy_read(phydev, ADIN1300_PHY_CTRL1);
+ if (reg < 0)
+ return reg;
+
+ auto_en = !!(reg & ADIN1300_AUTO_MDI_EN);
+ mdix_en = !!(reg & ADIN1300_MAN_MDIX_EN);
+
+ /* If MDI/MDIX is forced, just read it from the control reg */
+ if (!auto_en) {
+ if (mdix_en)
+ phydev->mdix = ETH_TP_MDI_X;
+ else
+ phydev->mdix = ETH_TP_MDI;
+ return 0;
+ }
+
+ /**
+* Otherwise, we need to deduce it from the PHY status2 reg.
+* When Auto-MDI is enabled, the ADIN1300_MAN_MDIX_EN bit implies
+* a preference for MDIX when it is set.
+*/
+ reg = phy_read(phydev, ADIN1300_PHY_STATUS1);
+ if (reg < 0)
+ return reg;
+
+ swapped = !!(reg & ADIN1300_PAIR_01_SWAP);
+
+ if (mdix_en != swapped)
+ phydev->mdix
[PATCH 02/16] net: phy: adin: hook genphy_{suspend,resume} into the driver
The chip supports standard suspend/resume via BMCR reg.
Hook these functions into the `adin` driver.
Signed-off-by: Alexandru Ardelean
---
drivers/net/phy/adin.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 6a610d4563c3..c100a0dd95cd 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -34,6 +34,8 @@ static struct phy_driver adin_driver[] = {
.config_init= adin_config_init,
.config_aneg= genphy_config_aneg,
.read_status= genphy_read_status,
+ .resume = genphy_resume,
+ .suspend= genphy_suspend,
},
{
.phy_id = PHY_ID_ADIN1300,
@@ -43,6 +45,8 @@ static struct phy_driver adin_driver[] = {
.config_init= adin_config_init,
.config_aneg= genphy_config_aneg,
.read_status= genphy_read_status,
+ .resume = genphy_resume,
+ .suspend= genphy_suspend,
},
};
--
2.20.1
[PATCH 00/16] net: phy: adin: add support for Analog Devices PHYs
This changeset adds support for Analog Devices Industrial Ethernet PHYs.
Particularly the PHYs this driver adds support for:
* ADIN1200 - Robust, Industrial, Low Power 10/100 Ethernet PHY
* ADIN1300 - Robust, Industrial, Low Latency 10/100/1000 Gigabit
Ethernet PHY
The 2 chips are pin & register compatible with one another. The main
difference being that ADIN1200 doesn't operate in gigabit mode.
The chips can be operated by the Generic PHY driver as well via the
standard IEEE PHY registers (0x - 0x000F) which are supported by the
kernel as well. This assumes that configuration of the PHY has been done
completely in HW, according to spec, i.e. no extra SW configuration
required.
This changeset also implements the ability to configure the chips via SW
registers.
Datasheets:
https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1300.pdf
https://www.analog.com/media/en/technical-documentation/data-sheets/ADIN1200.pdf
Signed-off-by: Alexandru Ardelean
Alexandru Ardelean (16):
net: phy: adin: add support for Analog Devices PHYs
net: phy: adin: hook genphy_{suspend,resume} into the driver
net: phy: adin: add support for interrupts
net: phy: adin: add {write,read}_mmd hooks
net: phy: adin: configure RGMII/RMII/MII modes on config
net: phy: adin: support PHY mode converters
net: phy: adin: make RGMII internal delays configurable
net: phy: adin: make RMII fifo depth configurable
net: phy: adin: add support MDI/MDIX/Auto-MDI selection
net: phy: adin: add EEE translation layer for Clause 22
net: phy: adin: PHY reset mechanisms
net: phy: adin: read EEE setting from device-tree
net: phy: adin: implement Energy Detect Powerdown mode
net: phy: adin: make sure down-speed auto-neg is enabled
net: phy: adin: add ethtool get_stats support
dt-bindings: net: add bindings for ADIN PHY driver
.../devicetree/bindings/net/adi,adin.yaml | 93 +++
MAINTAINERS | 9 +
drivers/net/phy/Kconfig | 9 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/adin.c| 752 ++
include/dt-bindings/net/adin.h| 26 +
6 files changed, 890 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/adi,adin.yaml
create mode 100644 drivers/net/phy/adin.c
create mode 100644 include/dt-bindings/net/adin.h
--
2.20.1
[PATCH 0/3][V4] iio: imu: Add support for the ADIS16460 IMU
This changeset adds support for the ADIS16460. The default CS change delay is 10 uS, while the ADIS16460 requires a minimum of 16 uS. As it turns out, the SPI framework support already has support for this feature, which is currently present in the for-next branch: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=0ff2de8bb163551ec4230a5a6f3c40c1f6adec4f This changeset now makes use of that feature, to allow longer CS change times (as needed) for ADIS16460. The SPI patch is present in the iio/testing branch, but not present in the iio/togreg branch. Changelog v3 -> v4: * for SPI: no change * for ADIS lib: use existing `cs_change_delay` feature (from SPI) * for DT: no change Changelog v2 -> v3: * for SPI: * used `cs_change_delay` instead of `cs_change_delay_usecs` (i.e. removed `_usecs` suffix * used SPI specific subject line for SPI patch * for ADIS lib: * updated to use the `cs_change_delay` * for DT: * added Rob's `Reviewed-by` tag Changelog v1 -> v2: * for SPI: * renamed `cs_change_stall_delay_us` -> `cs_change_delay_usecs` initial recommendation was `cs_change_delay`, but decided to name this `cs_change_delay_usecs`, since the convention for these delays seems to add the `_usecs` suffix * for ADIS lib: * renamed `stall_delay` -> `cs_change_delay` * removed some assignments of `cs_change_delay` where `cs_change` is not set * for ADIS16460 driver: * fixed license * adjusted to new `cs_change_delay[_usecs]` Alexandru Ardelean (3): iio: imu: adis: Add support for SPI transfer cs_change_delay iio: imu: Add support for the ADIS16460 IMU dt-bindings: iio: imu: add bindings for ADIS16460 .../bindings/iio/imu/adi,adis16460.yaml | 53 ++ MAINTAINERS | 8 + drivers/iio/imu/Kconfig | 12 + drivers/iio/imu/Makefile | 1 + drivers/iio/imu/adis.c| 12 + drivers/iio/imu/adis16460.c | 489 ++ include/linux/iio/imu/adis.h | 2 + 7 files changed, 577 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml create mode 100644 drivers/iio/imu/adis16460.c -- 2.20.1
[PATCH 1/3][V4] iio: imu: adis: Add support for SPI transfer cs_change_delay
The ADIS16460 requires a higher delay before the next transfer. Since the
SPI framework supports configuring the delay before the next transfer, this
driver will become the first user of it.
The support for this functionality in ADIS16460 requires an addition to the
ADIS lib to support the `cs_change_delay` functionality from the SPI
subsystem.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c | 12
include/linux/iio/imu/adis.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 30281e91dbf9..1631c255deab 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -39,18 +39,24 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
+ .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
+ .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 4,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
+ .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 6,
.bits_per_word = 8,
@@ -133,12 +139,16 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
+ .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
+ .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
}, {
.tx_buf = adis->tx + 4,
.rx_buf = adis->rx,
@@ -146,6 +156,8 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
+ .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
}, {
.rx_buf = adis->rx + 2,
.bits_per_word = 8,
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 3428d06b2f44..4c53815bb729 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -26,6 +26,7 @@ struct adis_burst;
* struct adis_data - ADIS chip variant specific data
* @read_delay: SPI delay for read operations in us
* @write_delay: SPI delay for write operations in us
+ * @cs_change_delay: SPI delay between CS changes in us
* @glob_cmd_reg: Register address of the GLOB_CMD register
* @msc_ctrl_reg: Register address of the MSC_CTRL register
* @diag_stat_reg: Register address of the DIAG_STAT register
@@ -35,6 +36,7 @@ struct adis_burst;
struct adis_data {
unsigned int read_delay;
unsigned int write_delay;
+ unsigned int cs_change_delay;
unsigned int glob_cmd_reg;
unsigned int msc_ctrl_reg;
--
2.20.1
[PATCH 3/3][V4] dt-bindings: iio: imu: add bindings for ADIS16460
This change adds device-tree bindings for the ADIS16460.
Reviewed-by: Rob Herring
Signed-off-by: Alexandru Ardelean
---
.../bindings/iio/imu/adi,adis16460.yaml | 53 +++
MAINTAINERS | 1 +
2 files changed, 54 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
new file mode 100644
index ..0c53009ba7d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/adi,adis16460.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIS16460 and similar IMUs
+
+maintainers:
+ - Dragos Bogdan
+
+description: |
+ Analog Devices ADIS16460 and similar IMUs
+
https://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16460.pdf
+
+properties:
+ compatible:
+enum:
+ - adi,adis16460
+
+ reg:
+maxItems: 1
+
+ spi-cpha: true
+
+ spi-cpol: true
+
+ interrupts:
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+examples:
+ - |
+#include
+#include
+spi0 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+imu@0 {
+compatible = "adi,adis16460";
+reg = <0>;
+spi-max-frequency = <500>;
+spi-cpol;
+spi-cpha;
+interrupt-parent = <&gpio0>;
+interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+};
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index f7de89e82e35..07105e43ea1e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -951,6 +951,7 @@ S: Supported
L: linux-...@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
F: drivers/iio/imu/adis16460.c
+F: Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
ANALOG DEVICES INC ADP5061 DRIVER
M: Stefan Popa
--
2.20.1
[PATCH 2/3][V4] iio: imu: Add support for the ADIS16460 IMU
The ADIS16460 device is a complete inertial system that includes a triaxial gyroscope and a triaxial accelerometer. It's more simplified design than that of the ADIS16480, and does not offer the triaxial magnetometers & pressure sensors. It does also have a temperature sensor (like the ADIS16480). Since it is part of the ADIS16XXX family, it re-uses parts of the ADIS library. Naturally, the register map is different and much more simplified than the ADIS16480 subfamily, so it cannot be integrated into that driver. A major difference is that the registers are not paged. One thing that is particularly special about it, is that it requires a higher delay between CS changes (i.e. when CS goes up, the spec recommends that it be brought down after a minimum of 16 uS). Other ADIS chips require (via spec) a minimum of 2 uS between CS changes. The kernel's 10 uS default should be fine for those other chips; they haven't been tested with lower CS change delays yet. Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adis16460.pdf Signed-off-by: Dragos Bogdan Signed-off-by: Michael Hennerich Signed-off-by: Alexandru Ardelean --- MAINTAINERS | 7 + drivers/iio/imu/Kconfig | 12 + drivers/iio/imu/Makefile| 1 + drivers/iio/imu/adis16460.c | 489 4 files changed, 509 insertions(+) create mode 100644 drivers/iio/imu/adis16460.c diff --git a/MAINTAINERS b/MAINTAINERS index 783569e3c4b4..f7de89e82e35 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -945,6 +945,13 @@ L: linux-...@vger.kernel.org F: include/linux/iio/imu/adis.h F: drivers/iio/imu/adis.c +ANALOG DEVICES INC ADIS16460 DRIVER +M: Dragos Bogdan +S: Supported +L: linux-...@vger.kernel.org +W: http://ez.analog.com/community/linux-device-drivers +F: drivers/iio/imu/adis16460.c + ANALOG DEVICES INC ADP5061 DRIVER M: Stefan Popa L: linux...@vger.kernel.org diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig index 4957e6df447e..f3c7282321a8 100644 --- a/drivers/iio/imu/Kconfig +++ b/drivers/iio/imu/Kconfig @@ -17,6 +17,18 @@ config ADIS16400 adis16365, adis16400 and adis16405 triaxial inertial sensors (adis16400 series also have magnetometers). +config ADIS16460 + tristate "Analog Devices ADIS16460 and similar IMU driver" + depends on SPI + select IIO_ADIS_LIB + select IIO_ADIS_LIB_BUFFER if IIO_BUFFER + help + Say yes here to build support for Analog Devices ADIS16460 inertial + sensor. + + To compile this driver as a module, choose M here: the module will be + called adis16460. + config ADIS16480 tristate "Analog Devices ADIS16480 and similar IMU driver" depends on SPI diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile index 9e452fce1aaf..4a6958865504 100644 --- a/drivers/iio/imu/Makefile +++ b/drivers/iio/imu/Makefile @@ -5,6 +5,7 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_ADIS16400) += adis16400.o +obj-$(CONFIG_ADIS16460) += adis16460.o obj-$(CONFIG_ADIS16480) += adis16480.o adis_lib-y += adis.o diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c new file mode 100644 index ..db713cba75a2 --- /dev/null +++ b/drivers/iio/imu/adis16460.c @@ -0,0 +1,489 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ADIS16460 IMU driver + * + * Copyright 2019 Analog Devices Inc. + */ + +#include +#include +#include + +#include +#include + +#include + +#define ADIS16460_REG_FLASH_CNT0x00 +#define ADIS16460_REG_DIAG_STAT0x02 +#define ADIS16460_REG_X_GYRO_LOW 0x04 +#define ADIS16460_REG_X_GYRO_OUT 0x06 +#define ADIS16460_REG_Y_GYRO_LOW 0x08 +#define ADIS16460_REG_Y_GYRO_OUT 0x0A +#define ADIS16460_REG_Z_GYRO_LOW 0x0C +#define ADIS16460_REG_Z_GYRO_OUT 0x0E +#define ADIS16460_REG_X_ACCL_LOW 0x10 +#define ADIS16460_REG_X_ACCL_OUT 0x12 +#define ADIS16460_REG_Y_ACCL_LOW 0x14 +#define ADIS16460_REG_Y_ACCL_OUT 0x16 +#define ADIS16460_REG_Z_ACCL_LOW 0x18 +#define ADIS16460_REG_Z_ACCL_OUT 0x1A +#define ADIS16460_REG_SMPL_CNTR0x1C +#define ADIS16460_REG_TEMP_OUT 0x1E +#define ADIS16460_REG_X_DELT_ANG 0x24 +#define ADIS16460_REG_Y_DELT_ANG 0x26 +#define ADIS16460_REG_Z_DELT_ANG 0x28 +#define ADIS16460_REG_X_DELT_VEL 0x2A +#define ADIS16460_REG_Y_DELT_VEL 0x2C +#define ADIS16460_REG_Z_DELT_VEL 0x2E +#define ADIS16460_REG_MSC_CTRL 0x32 +#define ADIS16460_REG_SYNC_SCAL0x34 +#define ADIS16460_REG_DEC_RATE 0x36 +#define ADIS16460_REG_FLTR_CTRL0x38 +#define ADIS16460_REG_GLOB_CMD 0x3E +#define ADIS16460_REG_X_GYRO_OFF 0x40 +#define ADIS16460_REG_Y_GYRO_OFF 0x42 +#define ADIS1
Re: [PATCH 1/4][V3] spi: Add optional stall delay between cs_change transfers
On Mon, Jul 22, 2019 at 8:42 PM Mark Brown wrote: > > On Mon, Jul 22, 2019 at 03:47:44PM +0300, Alexandru Ardelean wrote: > > Some devices like the ADIS16460 IMU require a longer period between > > transfers, i.e. between when the CS is de-asserted and re-asserted. The > > default value of 10us is not enough. This change makes the delay > > configurable for when the next CS change goes active, allowing the default > > to remain 10us is case it is unspecified. > > For the third time: > > | This looks like cs_change_delay. > > > #define SPI_NBITS_QUAD 0x04 /* 4bits transfer */ > > u8 bits_per_word; > > u8 word_delay_usecs; > > + u8 cs_change_delay; > > u16 delay_usecs; > > u32 speed_hz; > > u16 word_delay; > > This patch doesn't apply and even if it did it won't compile because you > are trying to add a field with the same name as an existing one. now i see; well, my fault here; i was basing my patchset on top of branch iio/togreg from Jonathan's tree for the IMU: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/log/?h=togreg [ typically that's the base branch for new IIO drivers ] that one is a bit behind, and does not contain the cs_change_delay stuff you mentioned; also, i will admit that sometimes, some review comments are not completely obvious to me; i should have checked the SPI tree before opening my mouth, but this will [hopefully] serve me as a learning experience when sending multi-subsystem patches when sending to a single subsystem, it's clear; when sending to 2, i get a bit lost i do feel a bit bad for the noise i caused, but it's not the worst thing i did today anyway: disregard this, and i will sync with Jonathan about how to proceed with this patch just for IIO; thanks for your time and sorry for the noise Alex
[PATCH 0/4][V3] iio: imu: Add support for the ADIS16460 IMU
This changeset adds support for the ADIS16460. Support for this chip, requires changes in both IIO & SPI, in order to support configurable/longer CS change delays. The default CS change delay is 10 uS, while the ADIS16460 requires a minimum of 16 uS. In order to accomodate this, the SPI transfer struct requires a `cs_change_delay_usecs` parameter that is used when `cs_change` is set. The ADIS library also requires a small update to support the new SPI `cs_change_delay_usecs`, and after that, support for ADIS16460 is added, since all the required parts for operating the chip are in the kernel. Continuing discussion from: https://lore.kernel.org/lkml/20190717115109.15168-5-alexandru.ardel...@analog.com/T/ Changelog v2 -> v3: * for SPI: * used `cs_change_delay` instead of `cs_change_delay_usecs` (i.e. removed `_usecs` suffix * used SPI specific subject line for SPI patch * for ADIS lib: * updated to use the `cs_change_delay` * for DT: * added Rob's `Reviewed-by` tag Changelog v1 -> v2: * for SPI: * renamed `cs_change_stall_delay_us` -> `cs_change_delay_usecs` initial recommendation was `cs_change_delay`, but decided to name this `cs_change_delay_usecs`, since the convention for these delays seems to add the `_usecs` suffix * for ADIS lib: * renamed `stall_delay` -> `cs_change_delay` * removed some assignments of `cs_change_delay` where `cs_change` is not set * for ADIS16460 driver: * fixed license * adjusted to new `cs_change_delay[_usecs]` Alexandru Ardelean (4): spi: Add optional stall delay between cs_change transfers iio: imu: adis: Add support for SPI transfer cs_change_delay iio: imu: Add support for the ADIS16460 IMU dt-bindings: iio: imu: add bindings for ADIS16460 .../bindings/iio/imu/adi,adis16460.yaml | 53 ++ MAINTAINERS | 8 + drivers/iio/imu/Kconfig | 12 + drivers/iio/imu/Makefile | 1 + drivers/iio/imu/adis.c| 6 + drivers/iio/imu/adis16460.c | 489 ++ drivers/spi/spi.c | 3 +- include/linux/iio/imu/adis.h | 2 + include/linux/spi/spi.h | 2 + 9 files changed, 575 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml create mode 100644 drivers/iio/imu/adis16460.c -- 2.20.1
[PATCH 3/4][V3] iio: imu: Add support for the ADIS16460 IMU
The ADIS16460 device is a complete inertial system that includes a triaxial gyroscope and a triaxial accelerometer. It's more simplified design than that of the ADIS16480, and does not offer the triaxial magnetometers & pressure sensors. It does also have a temperature sensor (like the ADIS16480). Since it is part of the ADIS16XXX family, it re-uses parts of the ADIS library. Naturally, the register map is different and much more simplified than the ADIS16480 subfamily, so it cannot be integrated into that driver. A major difference is that the registers are not paged. One thing that is particularly special about it, is that it requires a higher delay between CS changes (i.e. when CS goes up, the spec recommends that it be brought down after a minimum of 16 uS). Other ADIS chips require (via spec) a minimum of 2 uS between CS changes. The kernel's 10 uS default should be fine for those other chips; they haven't been tested with lower CS change delays yet. Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adis16460.pdf Signed-off-by: Dragos Bogdan Signed-off-by: Michael Hennerich Signed-off-by: Alexandru Ardelean --- MAINTAINERS | 7 + drivers/iio/imu/Kconfig | 12 + drivers/iio/imu/Makefile| 1 + drivers/iio/imu/adis16460.c | 489 4 files changed, 509 insertions(+) create mode 100644 drivers/iio/imu/adis16460.c diff --git a/MAINTAINERS b/MAINTAINERS index ad498428b38c..8e679504c087 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -937,6 +937,13 @@ L: linux-...@vger.kernel.org F: include/linux/iio/imu/adis.h F: drivers/iio/imu/adis.c +ANALOG DEVICES INC ADIS16460 DRIVER +M: Dragos Bogdan +S: Supported +L: linux-...@vger.kernel.org +W: http://ez.analog.com/community/linux-device-drivers +F: drivers/iio/imu/adis16460.c + ANALOG DEVICES INC ADP5061 DRIVER M: Stefan Popa L: linux...@vger.kernel.org diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig index 156630a21696..f048d0d757b1 100644 --- a/drivers/iio/imu/Kconfig +++ b/drivers/iio/imu/Kconfig @@ -16,6 +16,18 @@ config ADIS16400 adis16365, adis16400 and adis16405 triaxial inertial sensors (adis16400 series also have magnetometers). +config ADIS16460 + tristate "Analog Devices ADIS16460 and similar IMU driver" + depends on SPI + select IIO_ADIS_LIB + select IIO_ADIS_LIB_BUFFER if IIO_BUFFER + help + Say yes here to build support for Analog Devices ADIS16460 inertial + sensor. + + To compile this driver as a module, choose M here: the module will be + called adis16460. + config ADIS16480 tristate "Analog Devices ADIS16480 and similar IMU driver" depends on SPI diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile index 9e452fce1aaf..4a6958865504 100644 --- a/drivers/iio/imu/Makefile +++ b/drivers/iio/imu/Makefile @@ -5,6 +5,7 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_ADIS16400) += adis16400.o +obj-$(CONFIG_ADIS16460) += adis16460.o obj-$(CONFIG_ADIS16480) += adis16480.o adis_lib-y += adis.o diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c new file mode 100644 index ..db713cba75a2 --- /dev/null +++ b/drivers/iio/imu/adis16460.c @@ -0,0 +1,489 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ADIS16460 IMU driver + * + * Copyright 2019 Analog Devices Inc. + */ + +#include +#include +#include + +#include +#include + +#include + +#define ADIS16460_REG_FLASH_CNT0x00 +#define ADIS16460_REG_DIAG_STAT0x02 +#define ADIS16460_REG_X_GYRO_LOW 0x04 +#define ADIS16460_REG_X_GYRO_OUT 0x06 +#define ADIS16460_REG_Y_GYRO_LOW 0x08 +#define ADIS16460_REG_Y_GYRO_OUT 0x0A +#define ADIS16460_REG_Z_GYRO_LOW 0x0C +#define ADIS16460_REG_Z_GYRO_OUT 0x0E +#define ADIS16460_REG_X_ACCL_LOW 0x10 +#define ADIS16460_REG_X_ACCL_OUT 0x12 +#define ADIS16460_REG_Y_ACCL_LOW 0x14 +#define ADIS16460_REG_Y_ACCL_OUT 0x16 +#define ADIS16460_REG_Z_ACCL_LOW 0x18 +#define ADIS16460_REG_Z_ACCL_OUT 0x1A +#define ADIS16460_REG_SMPL_CNTR0x1C +#define ADIS16460_REG_TEMP_OUT 0x1E +#define ADIS16460_REG_X_DELT_ANG 0x24 +#define ADIS16460_REG_Y_DELT_ANG 0x26 +#define ADIS16460_REG_Z_DELT_ANG 0x28 +#define ADIS16460_REG_X_DELT_VEL 0x2A +#define ADIS16460_REG_Y_DELT_VEL 0x2C +#define ADIS16460_REG_Z_DELT_VEL 0x2E +#define ADIS16460_REG_MSC_CTRL 0x32 +#define ADIS16460_REG_SYNC_SCAL0x34 +#define ADIS16460_REG_DEC_RATE 0x36 +#define ADIS16460_REG_FLTR_CTRL0x38 +#define ADIS16460_REG_GLOB_CMD 0x3E +#define ADIS16460_REG_X_GYRO_OFF 0x40 +#define ADIS16460_REG_Y_GYRO_OFF 0x42 +#define ADIS1
[PATCH 4/4][V3] dt-bindings: iio: imu: add bindings for ADIS16460
This change adds device-tree bindings for the ADIS16460.
Reviewed-by: Rob Herring
Signed-off-by: Alexandru Ardelean
---
.../bindings/iio/imu/adi,adis16460.yaml | 53 +++
MAINTAINERS | 1 +
2 files changed, 54 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
new file mode 100644
index ..0c53009ba7d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/adi,adis16460.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIS16460 and similar IMUs
+
+maintainers:
+ - Dragos Bogdan
+
+description: |
+ Analog Devices ADIS16460 and similar IMUs
+
https://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16460.pdf
+
+properties:
+ compatible:
+enum:
+ - adi,adis16460
+
+ reg:
+maxItems: 1
+
+ spi-cpha: true
+
+ spi-cpol: true
+
+ interrupts:
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+examples:
+ - |
+#include
+#include
+spi0 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+imu@0 {
+compatible = "adi,adis16460";
+reg = <0>;
+spi-max-frequency = <500>;
+spi-cpol;
+spi-cpha;
+interrupt-parent = <&gpio0>;
+interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+};
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 8e679504c087..c44fbe8e91e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -943,6 +943,7 @@ S: Supported
L: linux-...@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
F: drivers/iio/imu/adis16460.c
+F: Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
ANALOG DEVICES INC ADP5061 DRIVER
M: Stefan Popa
--
2.20.1
[PATCH 2/4][V3] iio: imu: adis: Add support for SPI transfer cs_change_delay
The ADIS16460 requires a higher delay before the next transfer. Since the
SPI framework supports configuring the delay before the next transfer, this
driver will become the first user of it.
The support for this functionality in ADIS16460 requires an addition to the
ADIS lib to support the `cs_change_delay` functionality in SPI.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c | 6 ++
include/linux/iio/imu/adis.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index c771ae6803a9..47f64a6bbc69 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -40,18 +40,21 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 4,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 6,
.bits_per_word = 8,
@@ -134,12 +137,14 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 4,
.rx_buf = adis->rx,
@@ -147,6 +152,7 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
+ .cs_change_delay = adis->data->cs_change_delay,
}, {
.rx_buf = adis->rx + 2,
.bits_per_word = 8,
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 469a493f7ae0..fd884b45ed45 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -27,6 +27,7 @@ struct adis_burst;
* struct adis_data - ADIS chip variant specific data
* @read_delay: SPI delay for read operations in us
* @write_delay: SPI delay for write operations in us
+ * @cs_change_delay: SPI delay between CS changes in us
* @glob_cmd_reg: Register address of the GLOB_CMD register
* @msc_ctrl_reg: Register address of the MSC_CTRL register
* @diag_stat_reg: Register address of the DIAG_STAT register
@@ -36,6 +37,7 @@ struct adis_burst;
struct adis_data {
unsigned int read_delay;
unsigned int write_delay;
+ unsigned int cs_change_delay;
unsigned int glob_cmd_reg;
unsigned int msc_ctrl_reg;
--
2.20.1
[PATCH 1/4][V3] spi: Add optional stall delay between cs_change transfers
Some devices like the ADIS16460 IMU require a longer period between
transfers, i.e. between when the CS is de-asserted and re-asserted. The
default value of 10us is not enough. This change makes the delay
configurable for when the next CS change goes active, allowing the default
to remain 10us is case it is unspecified.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi.c | 3 ++-
include/linux/spi/spi.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 5e75944ad5d1..fe7fa7fb25c5 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1163,7 +1163,8 @@ static int spi_transfer_one_message(struct spi_controller
*ctlr,
keep_cs = true;
} else {
spi_set_cs(msg->spi, false);
- udelay(10);
+ udelay(xfer->cs_change_delay ?
+ xfer->cs_change_delay : 10);
spi_set_cs(msg->spi, true);
}
}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 053abd22ad31..6de287f8e465 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -734,6 +734,7 @@ extern void spi_res_release(struct spi_controller *ctlr,
* transfer. If 0 the default (from @spi_device) is used.
* @bits_per_word: select a bits_per_word other than the device default
* for this transfer. If 0 the default (from @spi_device) is used.
+ * @cs_change_delay: microseconds to delay between cs_change transfers.
* @cs_change: affects chipselect after this transfer completes
* @delay_usecs: microseconds to delay after this transfer before
* (optionally) changing the chipselect status, then starting
@@ -823,6 +824,7 @@ struct spi_transfer {
#defineSPI_NBITS_QUAD 0x04 /* 4bits transfer */
u8 bits_per_word;
u8 word_delay_usecs;
+ u8 cs_change_delay;
u16 delay_usecs;
u32 speed_hz;
u16 word_delay;
--
2.20.1
[PATCH 0/4][V2] iio: imu: Add support for the ADIS16460 IMU
This changeset adds support for the ADIS16460. Support for this chip, requires changes in both IIO & SPI, in order to support configurable/longer CS change delays. The default CS change delay is 10 uS, while the ADIS16460 requires a minimum of 16 uS. In order to accomodate this, the SPI transfer struct requires a `cs_change_delay_usecs` parameter that is used when `cs_change` is set. The ADIS library also requires a small update to support the new SPI `cs_change_delay_usecs`, and after that, support for ADIS16460 is added, since all the required parts for operating the chip are in the kernel. Changelog v1 -> v2: * for SPI: * renamed `cs_change_stall_delay_us` -> `cs_change_delay_usecs` initial recommendation was `cs_change_delay`, but decided to name this `cs_change_delay_usecs`, since the convention for these delays seems to add the `_usecs` suffix * for ADIS lib: * renamed `stall_delay` -> `cs_change_delay` * removed some assignments of `cs_change_delay` where `cs_change` is not set * for ADIS16460 driver: * fixed license * adjusted to new `cs_change_delay[_usecs]` Alexandru Ardelean (4): drivers: spi: core: Add optional stall delay between cs_change transfers iio: imu: adis: Add support for SPI transfer cs_change_stall_delay_us iio: imu: Add support for the ADIS16460 IMU dt-bindings: iio: imu: add bindings for ADIS16460 .../bindings/iio/imu/adi,adis16460.yaml | 53 ++ MAINTAINERS | 8 + drivers/iio/imu/Kconfig | 12 + drivers/iio/imu/Makefile | 1 + drivers/iio/imu/adis.c| 6 + drivers/iio/imu/adis16460.c | 489 ++ drivers/spi/spi.c | 3 +- include/linux/iio/imu/adis.h | 2 + include/linux/spi/spi.h | 3 + 9 files changed, 576 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml create mode 100644 drivers/iio/imu/adis16460.c -- 2.20.1
[PATCH 4/4][V2] dt-bindings: iio: imu: add bindings for ADIS16460
This change adds device-tree bindings for the ADIS16460.
Signed-off-by: Alexandru Ardelean
---
.../bindings/iio/imu/adi,adis16460.yaml | 53 +++
MAINTAINERS | 1 +
2 files changed, 54 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
new file mode 100644
index ..0c53009ba7d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/adi,adis16460.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADIS16460 and similar IMUs
+
+maintainers:
+ - Dragos Bogdan
+
+description: |
+ Analog Devices ADIS16460 and similar IMUs
+
https://www.analog.com/media/en/technical-documentation/data-sheets/ADIS16460.pdf
+
+properties:
+ compatible:
+enum:
+ - adi,adis16460
+
+ reg:
+maxItems: 1
+
+ spi-cpha: true
+
+ spi-cpol: true
+
+ interrupts:
+maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+examples:
+ - |
+#include
+#include
+spi0 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+imu@0 {
+compatible = "adi,adis16460";
+reg = <0>;
+spi-max-frequency = <500>;
+spi-cpol;
+spi-cpha;
+interrupt-parent = <&gpio0>;
+interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+};
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index 8e679504c087..c44fbe8e91e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -943,6 +943,7 @@ S: Supported
L: linux-...@vger.kernel.org
W: http://ez.analog.com/community/linux-device-drivers
F: drivers/iio/imu/adis16460.c
+F: Documentation/devicetree/bindings/iio/imu/adi,adis16460.yaml
ANALOG DEVICES INC ADP5061 DRIVER
M: Stefan Popa
--
2.20.1
[PATCH 2/4][V2] iio: imu: adis: Add support for SPI transfer cs_change_delay_usecs
The ADIS16460 requires a higher delay before the next transfer. Since the
SPI framework supports configuring the delay before the next transfer, this
driver will become the first user of it.
The support for this functionality in ADIS16460 requires an addition to the
ADIS lib to support the `cs_change_stall_delay_us` functionality in SPI.
Not all transfers set `cs_change` to 1. Only those that do, have the
`cs_change_delay` assigned.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c | 6 ++
include/linux/iio/imu/adis.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index c771ae6803a9..6fdb6f4cebd4 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -40,18 +40,21 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay_usecs = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay_usecs = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 4,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay_usecs = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 6,
.bits_per_word = 8,
@@ -134,12 +137,14 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_delay_usecs = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
+ .cs_change_delay_usecs = adis->data->cs_change_delay,
}, {
.tx_buf = adis->tx + 4,
.rx_buf = adis->rx,
@@ -147,6 +152,7 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
+ .cs_change_delay_usecs = adis->data->cs_change_delay,
}, {
.rx_buf = adis->rx + 2,
.bits_per_word = 8,
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 469a493f7ae0..fd884b45ed45 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -27,6 +27,7 @@ struct adis_burst;
* struct adis_data - ADIS chip variant specific data
* @read_delay: SPI delay for read operations in us
* @write_delay: SPI delay for write operations in us
+ * @cs_change_delay: SPI delay between CS changes in us
* @glob_cmd_reg: Register address of the GLOB_CMD register
* @msc_ctrl_reg: Register address of the MSC_CTRL register
* @diag_stat_reg: Register address of the DIAG_STAT register
@@ -36,6 +37,7 @@ struct adis_burst;
struct adis_data {
unsigned int read_delay;
unsigned int write_delay;
+ unsigned int cs_change_delay;
unsigned int glob_cmd_reg;
unsigned int msc_ctrl_reg;
--
2.20.1
[PATCH 3/4][V2] iio: imu: Add support for the ADIS16460 IMU
The ADIS16460 device is a complete inertial system that includes a triaxial gyroscope and a triaxial accelerometer. It's more simplified design than that of the ADIS16480, and does not offer the triaxial magnetometers & pressure sensors. It does also have a temperature sensor (like the ADIS16480). Since it is part of the ADIS16XXX family, it re-uses parts of the ADIS library. Naturally, the register map is different and much more simplified than the ADIS16480 subfamily, so it cannot be integrated into that driver. A major difference is that the registers are not paged. One thing that is particularly special about it, is that it requires a higher delay between CS changes (i.e. when CS goes up, the spec recommends that it be brought down after a minimum of 16 uS). Other ADIS chips require (via spec) a minimum of 2 uS between CS changes. The kernel's 10 uS default should be fine for those other chips; they haven't been tested with lower CS change delays yet. Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adis16460.pdf Signed-off-by: Dragos Bogdan Signed-off-by: Michael Hennerich Signed-off-by: Alexandru Ardelean --- MAINTAINERS | 7 + drivers/iio/imu/Kconfig | 12 + drivers/iio/imu/Makefile| 1 + drivers/iio/imu/adis16460.c | 489 4 files changed, 509 insertions(+) create mode 100644 drivers/iio/imu/adis16460.c diff --git a/MAINTAINERS b/MAINTAINERS index ad498428b38c..8e679504c087 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -937,6 +937,13 @@ L: linux-...@vger.kernel.org F: include/linux/iio/imu/adis.h F: drivers/iio/imu/adis.c +ANALOG DEVICES INC ADIS16460 DRIVER +M: Dragos Bogdan +S: Supported +L: linux-...@vger.kernel.org +W: http://ez.analog.com/community/linux-device-drivers +F: drivers/iio/imu/adis16460.c + ANALOG DEVICES INC ADP5061 DRIVER M: Stefan Popa L: linux...@vger.kernel.org diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig index 156630a21696..f048d0d757b1 100644 --- a/drivers/iio/imu/Kconfig +++ b/drivers/iio/imu/Kconfig @@ -16,6 +16,18 @@ config ADIS16400 adis16365, adis16400 and adis16405 triaxial inertial sensors (adis16400 series also have magnetometers). +config ADIS16460 + tristate "Analog Devices ADIS16460 and similar IMU driver" + depends on SPI + select IIO_ADIS_LIB + select IIO_ADIS_LIB_BUFFER if IIO_BUFFER + help + Say yes here to build support for Analog Devices ADIS16460 inertial + sensor. + + To compile this driver as a module, choose M here: the module will be + called adis16460. + config ADIS16480 tristate "Analog Devices ADIS16480 and similar IMU driver" depends on SPI diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile index 9e452fce1aaf..4a6958865504 100644 --- a/drivers/iio/imu/Makefile +++ b/drivers/iio/imu/Makefile @@ -5,6 +5,7 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_ADIS16400) += adis16400.o +obj-$(CONFIG_ADIS16460) += adis16460.o obj-$(CONFIG_ADIS16480) += adis16480.o adis_lib-y += adis.o diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c new file mode 100644 index ..db713cba75a2 --- /dev/null +++ b/drivers/iio/imu/adis16460.c @@ -0,0 +1,489 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ADIS16460 IMU driver + * + * Copyright 2019 Analog Devices Inc. + */ + +#include +#include +#include + +#include +#include + +#include + +#define ADIS16460_REG_FLASH_CNT0x00 +#define ADIS16460_REG_DIAG_STAT0x02 +#define ADIS16460_REG_X_GYRO_LOW 0x04 +#define ADIS16460_REG_X_GYRO_OUT 0x06 +#define ADIS16460_REG_Y_GYRO_LOW 0x08 +#define ADIS16460_REG_Y_GYRO_OUT 0x0A +#define ADIS16460_REG_Z_GYRO_LOW 0x0C +#define ADIS16460_REG_Z_GYRO_OUT 0x0E +#define ADIS16460_REG_X_ACCL_LOW 0x10 +#define ADIS16460_REG_X_ACCL_OUT 0x12 +#define ADIS16460_REG_Y_ACCL_LOW 0x14 +#define ADIS16460_REG_Y_ACCL_OUT 0x16 +#define ADIS16460_REG_Z_ACCL_LOW 0x18 +#define ADIS16460_REG_Z_ACCL_OUT 0x1A +#define ADIS16460_REG_SMPL_CNTR0x1C +#define ADIS16460_REG_TEMP_OUT 0x1E +#define ADIS16460_REG_X_DELT_ANG 0x24 +#define ADIS16460_REG_Y_DELT_ANG 0x26 +#define ADIS16460_REG_Z_DELT_ANG 0x28 +#define ADIS16460_REG_X_DELT_VEL 0x2A +#define ADIS16460_REG_Y_DELT_VEL 0x2C +#define ADIS16460_REG_Z_DELT_VEL 0x2E +#define ADIS16460_REG_MSC_CTRL 0x32 +#define ADIS16460_REG_SYNC_SCAL0x34 +#define ADIS16460_REG_DEC_RATE 0x36 +#define ADIS16460_REG_FLTR_CTRL0x38 +#define ADIS16460_REG_GLOB_CMD 0x3E +#define ADIS16460_REG_X_GYRO_OFF 0x40 +#define ADIS16460_REG_Y_GYRO_OFF 0x42 +#define ADIS1
[PATCH 1/4][V2] drivers: spi: core: Add optional delay between cs_change transfers
Some devices like the ADIS16460 IMU require a stall period between
transfers, i.e. between when the CS is de-asserted and re-asserted. The
default value of 10us is not enough. This change makes the delay
configurable for when the next CS change goes active.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi.c | 3 ++-
include/linux/spi/spi.h | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 5e75944ad5d1..02fd00bcaace 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1163,7 +1163,8 @@ static int spi_transfer_one_message(struct spi_controller
*ctlr,
keep_cs = true;
} else {
spi_set_cs(msg->spi, false);
- udelay(10);
+ udelay(xfer->cs_change_delay_usecs ?
+ xfer->cs_change_delay_usecs : 10);
spi_set_cs(msg->spi, true);
}
}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 053abd22ad31..c884b3b94841 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -734,6 +734,8 @@ extern void spi_res_release(struct spi_controller *ctlr,
* transfer. If 0 the default (from @spi_device) is used.
* @bits_per_word: select a bits_per_word other than the device default
* for this transfer. If 0 the default (from @spi_device) is used.
+ * @cs_change_delay_usecs: microseconds to delay between cs_change
+ * transfers.
* @cs_change: affects chipselect after this transfer completes
* @delay_usecs: microseconds to delay after this transfer before
* (optionally) changing the chipselect status, then starting
@@ -823,6 +825,7 @@ struct spi_transfer {
#defineSPI_NBITS_QUAD 0x04 /* 4bits transfer */
u8 bits_per_word;
u8 word_delay_usecs;
+ u8 cs_change_delay_usecs;
u16 delay_usecs;
u32 speed_hz;
u16 word_delay;
--
2.20.1
[PATCH][V4] lib: fix __sysfs_match_string() helper when n != -1
The documentation the `__sysfs_match_string()` helper mentions that `n`
(the size of the given array) should be:
* @n: number of strings in the array or -1 for NULL terminated arrays
The behavior of the function is different, in the sense that it exits on
the first NULL element in the array.
This patch changes the behavior, to exit the loop when a NULL element is
found, and the size of the array is provided as -1.
All current users of __sysfs_match_string() & sysfs_match_string() provide
contiguous arrays of strings, so this behavior change doesn't influence
anything (at this point in time).
This behavior change allows for an array of strings to have NULL elements
within the array, which will be ignored. This is particularly useful when
creating mapping of strings and integers (as bitfields or other HW
description).
Signed-off-by: Alexandru Ardelean
---
Changelog v3 -> v4:
* split this patch away from series; there are some unsolved discussions
that will probably need resolving per sub-system
lib/string.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/string.c b/lib/string.c
index 3ab861c1a857..5bea3f98478a 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -674,8 +674,11 @@ int __sysfs_match_string(const char * const *array, size_t
n, const char *str)
for (index = 0; index < n; index++) {
item = array[index];
- if (!item)
+ if (!item) {
+ if (n != (size_t)-1)
+ continue;
break;
+ }
if (sysfs_streq(item, str))
return index;
}
--
2.20.1
[PATCH 1/5] MAINTAINERS: add ADIS IMU driver library entry
This change adds the ADIS driver library to the MAINTAINERS list, and adds myself as the current maintainer of this library. Signed-off-by: Alexandru Ardelean --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 1eb971608ac4..544e23753e96 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -930,6 +930,13 @@ S: Supported F: drivers/mux/adgs1408.c F: Documentation/devicetree/bindings/mux/adi,adgs1408.txt +ANALOG DEVICES INC ADIS DRIVER LIBRARY +M: Alexandru Ardelean +S: Supported +L: linux-...@vger.kernel.org +F: include/linux/iio/imu/adis.h +F: drivers/iio/imu/adis.c + ANALOG DEVICES INC ADP5061 DRIVER M: Stefan Popa L: linux...@vger.kernel.org -- 2.20.1
[PATCH 3/5] iio: imu: adis: Add support for SPI transfer cs_change_stall_delay_us
The ADIS16460 requires a higher delay before the next transfer. Since the
SPI framework supports configuring the delay before the next transfer, this
driver will become the first user of it.
The support for this functionality in ADIS16460 requires an addition to the
ADIS lib to support the `cs_change_stall_delay_us` functionality in SPI.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis.c | 9 +
include/linux/iio/imu/adis.h | 2 ++
2 files changed, 11 insertions(+)
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index c771ae6803a9..90dac69910b3 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -40,28 +40,33 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
}, {
.tx_buf = adis->tx + 4,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
}, {
.tx_buf = adis->tx + 6,
.bits_per_word = 8,
.len = 2,
.delay_usecs = adis->data->write_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
}, {
.tx_buf = adis->tx + 8,
.bits_per_word = 8,
.len = 2,
.delay_usecs = adis->data->write_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
},
};
@@ -134,12 +139,14 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->write_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
}, {
.tx_buf = adis->tx + 2,
.bits_per_word = 8,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
}, {
.tx_buf = adis->tx + 4,
.rx_buf = adis->rx,
@@ -147,11 +154,13 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
.len = 2,
.cs_change = 1,
.delay_usecs = adis->data->read_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
}, {
.rx_buf = adis->rx + 2,
.bits_per_word = 8,
.len = 2,
.delay_usecs = adis->data->read_delay,
+ .cs_change_stall_delay_us = adis->data->cs_stall_delay,
},
};
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 469a493f7ae0..4aa248b6b3bd 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -27,6 +27,7 @@ struct adis_burst;
* struct adis_data - ADIS chip variant specific data
* @read_delay: SPI delay for read operations in us
* @write_delay: SPI delay for write operations in us
+ * @cs_stall_delay: SPI stall delay between transfers in us
* @glob_cmd_reg: Register address of the GLOB_CMD register
* @msc_ctrl_reg: Register address of the MSC_CTRL register
* @diag_stat_reg: Register address of the DIAG_STAT register
@@ -36,6 +37,7 @@ struct adis_burst;
struct adis_data {
unsigned int read_delay;
unsigned int write_delay;
+ unsigned int cs_stall_delay;
unsigned int glob_cmd_reg;
unsigned int msc_ctrl_reg;
--
2.20.1
[PATCH 2/5] drivers: spi: core: Add optional stall delay between cs_change transfers
Some devices like the ADIS16460 IMU require a stall period between
transfers, i.e. between when the CS is de-asserted and re-asserted. The
default value of 10us is not enough. This change makes the delay
configurable for when the next CS change goes active.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi.c | 3 ++-
include/linux/spi/spi.h | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 5e75944ad5d1..739de0118ee1 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1163,7 +1163,8 @@ static int spi_transfer_one_message(struct spi_controller
*ctlr,
keep_cs = true;
} else {
spi_set_cs(msg->spi, false);
- udelay(10);
+ udelay(xfer->cs_change_stall_delay_us ?
+ xfer->cs_change_stall_delay_us : 10);
spi_set_cs(msg->spi, true);
}
}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 053abd22ad31..d23add3b4790 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -734,6 +734,8 @@ extern void spi_res_release(struct spi_controller *ctlr,
* transfer. If 0 the default (from @spi_device) is used.
* @bits_per_word: select a bits_per_word other than the device default
* for this transfer. If 0 the default (from @spi_device) is used.
+ * @cs_change_stall_delay_us: microseconds to delay between cs_change
+ * transfers.
* @cs_change: affects chipselect after this transfer completes
* @delay_usecs: microseconds to delay after this transfer before
* (optionally) changing the chipselect status, then starting
@@ -823,6 +825,7 @@ struct spi_transfer {
#defineSPI_NBITS_QUAD 0x04 /* 4bits transfer */
u8 bits_per_word;
u8 word_delay_usecs;
+ u8 cs_change_stall_delay_us;
u16 delay_usecs;
u32 speed_hz;
u16 word_delay;
--
2.20.1
[PATCH 4/5] iio: imu: Add support for the ADIS16460 IMU
The ADIS16460 device is a complete inertial system that includes a triaxial gyroscope and a triaxial accelerometer. It's more simplified design than that of the ADIS16480, and does not offer the triaxial magnetometers & pressure sensors. It does also have a temperature sensor (like the ADIS16480). Since it is part of the ADIS16XXX family, it re-uses parts of the ADIS library. Naturally, the register map is different and much more simplified than the ADIS16480 subfamily, so it cannot be integrated into that driver. A major difference is that the registers are not paged. One thing that is particularly special about it, is that it requires a higher CS stall delay between data (around 16 uS vs other chips from the family requiring around 2 uS). Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/adis16460.pdf Signed-off-by: Dragos Bogdan Signed-off-by: Michael Hennerich Signed-off-by: Alexandru Ardelean --- MAINTAINERS | 7 + drivers/iio/imu/Kconfig | 9 + drivers/iio/imu/Makefile| 1 + drivers/iio/imu/adis16460.c | 490 4 files changed, 507 insertions(+) create mode 100644 drivers/iio/imu/adis16460.c diff --git a/MAINTAINERS b/MAINTAINERS index 544e23753e96..ef2e2cee32e1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -937,6 +937,13 @@ L: linux-...@vger.kernel.org F: include/linux/iio/imu/adis.h F: drivers/iio/imu/adis.c +ANALOG DEVICES INC ADIS16460 DRIVER +M: Dragos Bogdan +S: Supported +L: linux-...@vger.kernel.org +W: http://ez.analog.com/community/linux-device-drivers +F: drivers/iio/imu/adis16460.c + ANALOG DEVICES INC ADP5061 DRIVER M: Stefan Popa L: linux...@vger.kernel.org diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig index 156630a21696..a29a481c20d2 100644 --- a/drivers/iio/imu/Kconfig +++ b/drivers/iio/imu/Kconfig @@ -16,6 +16,15 @@ config ADIS16400 adis16365, adis16400 and adis16405 triaxial inertial sensors (adis16400 series also have magnetometers). +config ADIS16460 + tristate "Analog Devices ADIS16460 and similar IMU driver" + depends on SPI + select IIO_ADIS_LIB + select IIO_ADIS_LIB_BUFFER if IIO_BUFFER + help + Say yes here to build support for Analog Devices ADIS16460 inertial + sensor. + config ADIS16480 tristate "Analog Devices ADIS16480 and similar IMU driver" depends on SPI diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile index 9e452fce1aaf..4a6958865504 100644 --- a/drivers/iio/imu/Makefile +++ b/drivers/iio/imu/Makefile @@ -5,6 +5,7 @@ # When adding new entries keep the list in alphabetical order obj-$(CONFIG_ADIS16400) += adis16400.o +obj-$(CONFIG_ADIS16460) += adis16460.o obj-$(CONFIG_ADIS16480) += adis16480.o adis_lib-y += adis.o diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c new file mode 100644 index ..6c86af06b5d1 --- /dev/null +++ b/drivers/iio/imu/adis16460.c @@ -0,0 +1,490 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ADIS16460 IMU driver + * + * Copyright 2019 Analog Devices Inc. + * + */ + +#include +#include +#include + +#include +#include + +#include + +#define ADIS16460_REG_FLASH_CNT0x00 +#define ADIS16460_REG_DIAG_STAT0x02 +#define ADIS16460_REG_X_GYRO_LOW 0x04 +#define ADIS16460_REG_X_GYRO_OUT 0x06 +#define ADIS16460_REG_Y_GYRO_LOW 0x08 +#define ADIS16460_REG_Y_GYRO_OUT 0x0A +#define ADIS16460_REG_Z_GYRO_LOW 0x0C +#define ADIS16460_REG_Z_GYRO_OUT 0x0E +#define ADIS16460_REG_X_ACCL_LOW 0x10 +#define ADIS16460_REG_X_ACCL_OUT 0x12 +#define ADIS16460_REG_Y_ACCL_LOW 0x14 +#define ADIS16460_REG_Y_ACCL_OUT 0x16 +#define ADIS16460_REG_Z_ACCL_LOW 0x18 +#define ADIS16460_REG_Z_ACCL_OUT 0x1A +#define ADIS16460_REG_SMPL_CNTR0x1C +#define ADIS16460_REG_TEMP_OUT 0x1E +#define ADIS16460_REG_X_DELT_ANG 0x24 +#define ADIS16460_REG_Y_DELT_ANG 0x26 +#define ADIS16460_REG_Z_DELT_ANG 0x28 +#define ADIS16460_REG_X_DELT_VEL 0x2A +#define ADIS16460_REG_Y_DELT_VEL 0x2C +#define ADIS16460_REG_Z_DELT_VEL 0x2E +#define ADIS16460_REG_MSC_CTRL 0x32 +#define ADIS16460_REG_SYNC_SCAL0x34 +#define ADIS16460_REG_DEC_RATE 0x36 +#define ADIS16460_REG_FLTR_CTRL0x38 +#define ADIS16460_REG_GLOB_CMD 0x3E +#define ADIS16460_REG_X_GYRO_OFF 0x40 +#define ADIS16460_REG_Y_GYRO_OFF 0x42 +#define ADIS16460_REG_Z_GYRO_OFF 0x44 +#define ADIS16460_REG_X_ACCL_OFF 0x46 +#define ADIS16460_REG_Y_ACCL_OFF 0x48 +#define ADIS16460_REG_Z_ACCL_OFF 0x4A +#define ADIS16460_REG_LOT_ID1 R0x52 +#define ADIS16460_REG_LOT_ID2 R0x54 +#define ADIS16460_REG_PROD_ID 0x56 +#define ADIS16460_REG_S
Re: [PATCH] staging: iio: adis16240: add of_match_table entry
On Fri, May 24, 2019 at 6:30 AM Rodrigo Ribeiro wrote:
>
> This patch adds of_match_table entry in device driver in order to
> enable spi fallback probing.
>
> Signed-off-by: Rodrigo Ribeiro
> Reviewed-by: Marcelo Schmitt
> ---
> drivers/staging/iio/accel/adis16240.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/iio/accel/adis16240.c
> b/drivers/staging/iio/accel/adis16240.c
> index 8c6d23604eca..b80c8529784b 100644
> --- a/drivers/staging/iio/accel/adis16240.c
> +++ b/drivers/staging/iio/accel/adis16240.c
> @@ -444,6 +444,7 @@ MODULE_DEVICE_TABLE(of, adis16240_of_match);
> static struct spi_driver adis16240_driver = {
> .driver = {
> .name = "adis16240",
> + .of_match_table = adis16240_of_match,
This patch is missing the actual table.
> },
> .probe = adis16240_probe,
> .remove = adis16240_remove,
> --
> 2.20.1
>
Re: [PATCH] dt-bindings: iio: ad7949: switch binding to yaml
On Sun, May 19, 2019 at 8:29 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 19:40:36 -0300
> João Victor Marques de Oliveira wrote:
>
> > Changes switches from old text bindings, to YAML bindings, and also
> > include adi,reference-select property to specify the source for the
> > reference voltage signal.
> >
> > Signed-off-by: João Victor Marques de Oliveira
> >
> > Signed-off-by: Thiago L. A. Miller
> > Co-developed-by: Thiago L. A. Miller
> > Signed-off-by: Osvaldo M. Yasuda
> > Co-developed-by: Osvaldo M. Yasuda
> > ---
> > We're adding Charles-Antoine Couret as main dt maintainer since we have
> > just switched documentation to yaml format.
>
> Hmm. I'm not sure it makes sense to list you all as maintainers
> of this rather simple binding.
>
> We also just went through some changes on the reference handling so
> I think you are based on stale information here.
>
> Thanks,
>
> Jonathan
>
> >
> > .../devicetree/bindings/iio/adc/ad7949.txt| 16 -
> > .../devicetree/bindings/iio/adc/ad7949.yaml | 71 +++
> > 2 files changed, 71 insertions(+), 16 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/iio/adc/ad7949.txt
> > create mode 100644 Documentation/devicetree/bindings/iio/adc/ad7949.yaml
Maybe also update the MAINTAINERS file with this.
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> > b/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> > deleted file mode 100644
> > index c7f5057356b1..
> > --- a/Documentation/devicetree/bindings/iio/adc/ad7949.txt
> > +++ /dev/null
> > @@ -1,16 +0,0 @@
> > -* Analog Devices AD7949/AD7682/AD7689
> > -
> > -Required properties:
> > - - compatible: Should be one of
> > - * "adi,ad7949"
> > - * "adi,ad7682"
> > - * "adi,ad7689"
> > - - reg: spi chip select number for the device
> > - - vref-supply: The regulator supply for ADC reference voltage
> > -
> > -Example:
> > -adc@0 {
> > - compatible = "adi,ad7949";
> > - reg = <0>;
> > - vref-supply = <&vdd_supply>;
> > -};
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ad7949.yaml
> > b/Documentation/devicetree/bindings/iio/adc/ad7949.yaml
> > new file mode 100644
> > index ..111c9e26f8e7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ad7949.yaml
> > @@ -0,0 +1,71 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/ad7949.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +
nitpick: extra line
> > +title: Analog Devices AD7949/AD7682/AD7689
> > +
> > +maintainers:
> > + - Charles-Antoine Couret
> > + - João Victor Marques de Oliveira
> > + - Thiago L. A. Miller
> > + - Osvaldo M. Yasuda
> > +
> > +properties:
> > + compatible:
> > +enum:
> > + - adi,ad7949
> > + - adi,ad7682
> > + - adi,ad7689
> > +
> > + reg:
> > +description:
> > + spi chip select number for the device
this doesn't need a description
it's a standard property
> > +maxItems: 1
> > +
> > + vref-supply:
> > +description:
> > + The regulator supply for ADC reference voltage
> > +maxItems: 1
> > +
> > + adi,reference-select:
> > +enum: [0, 1, 2, 3, 6, 7]
> > +description:
> > +Select the reference voltage source to use when converting the
> > input voltages.
> > +0 - Internal 2.5V reference; temperature sensor enabled
> > +1 - Internal 4.096V reference; temperature sensor enabled
> > +2 - External reference, temperature sensor enabled, no buffer
> > +3 - External reference, temperature sensor enabled, buffer
> > enabled
> > +6 - External reference, temperature sensor disabled, no buffer
> > +7 - External reference, temperature sensor disabled, buffer
> > enabled
> This is changing...
>
> > +maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - vref-supply
> > +
> > +examples:
> > + - |
> > +spi0 {
> > +#address-cells = <0x1>;
> > +#size-cells = <0x0>;
> > +adc@0 {
> > +compatible = "adi,ad7949";
> > +reg = <0>;
> > +adi,reference-select = <0>;
> > +vref-supply = <&vdd_supply>;
> > +};
> > +};
One example is enough in this case.
They don't differ much.
> > + - |
> > +spi0 {
> > +#address-cells = <0x1>;
> > +#size-cells = <0x0>;
> > +adc@0 {
> > +compatible = "adi,ad7949";
> > +reg = <0>;
> > +adi,reference-select = <0>;
> > +};
> > +};
>
Re: [PATCH] dt-bindings: iio: accel: adxl372: switch to YAML bindings
On Sun, May 19, 2019 at 8:27 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 18:55:42 -0300
> Lucas Oshiro wrote:
>
> > Convert the old device tree documentation to yaml format.
> >
> > Signed-off-by: Lucas Oshiro
> > Signed-off-by: Rodrigo Ribeiro
> > Co-developed-by: Rodrigo Ribeiro
> > ---
> >
> > Hello,
> > We've added Stefan Popa as maintainer of the yaml documentation of this
> > driver
> > because we found through git that he was the author of the older
> > documentation.
>
> Definitely going to need an Ack from Stefan for that ;)
CC-ing my work-email
There are some issues with it and mailing lists; I'll hopefully sort
them out in the next weeks.
Stefan is out-of-office.
He'll take a look when he comes back.
I'll add a few notes until then.
I'd still like Stefan's ack to be final.
>
> I've not really gotten yaml formats into my head yet, but from a quick
> look I think this is fine. I will however be looking for review from others
> on these.
>
> Thanks,
>
> Jonathan
>
> >
> > .../bindings/iio/accel/adi,adxl372.yaml | 66 +++
> > .../devicetree/bindings/iio/accel/adxl372.txt | 33 --
> > 2 files changed, 66 insertions(+), 33 deletions(-)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/accel/adi,adxl372.yaml
> > delete mode 100644 Documentation/devicetree/bindings/iio/accel/adxl372.txt
Also update the MAINTAINERS file when changing this.
For reference, many things can be borrowed from the ADXL345, which is
similar (from a dt-binding doc perspective only).
> >
> > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adxl372.yaml
> > b/Documentation/devicetree/bindings/iio/accel/adi,adxl372.yaml
> > new file mode 100644
> > index ..a6e2893d2ab1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adxl372.yaml
> > @@ -0,0 +1,66 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/accelerometers/adi,adxl372.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices ADXL372 3-Axis, +/-(200g) Digital Accelerometer
> > +
> > +maintainers:
> > + - Stefan Popa
> > +
> > +description: |
> > + Analog Devices ADXL372 3-Axis, +/-(200g) Digital Accelerometer that
> > supports
> > + both I2C & SPI interfaces
> > +https://www.analog.com/en/products/adxl372.html
> > +
> > +properties:
> > + compatible:
> > +enum:
> > + - adi,adxl372
> > +
> > + reg:
> > +description: the I2C address or SPI chip select number for the device
no need to add a description for reg
it's a standard property
> > +maxItems: 1
> > +
> > + interrupts:
> > +description:
> > + interrupt mapping for IRQ as documented in
> > + Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
no need to describe this either
> > +maxItems: 1
> > +
> > +required:
> > + - compatible
> > + - reg
I think interrupts is also required.
> > +
> > +examples:
> > + - |
> > +#include
> > +#include
> > +i2c0 {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +/* Example for a I2C device node */
> > +accelerometer@53 {
> > +compatible = "adi,adxl372";
> > +reg = <0x53>;
> > +interrupt-parent = <&gpio>;
> > +interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> > +};
> > +};
> > + - |
> > +#include
> > +#include
> > +spi0 {
> > +#address-cells = <1>;
> > +#size-cells = <0>;
> > +
> > +accelerometer@0 {
> > +compatible = "adi,adxl372";
> > +reg = <0>;
> > +spi-max-frequency = <100>;
> > +interrupt-parent = <&gpio>;
> > +interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> > +};
> > +};
Rest looks good.
> > diff --git a/Documentation/devicetree/bindings/iio/accel/adxl372.txt
> > b/Documentation/devicetree/bindings/iio/accel/adxl372.txt
> > deleted file mode 100644
> > index a289964756a7..
> > --- a/Documentation/devicetree/bindings/iio/accel/adxl372.txt
> > +++ /dev/null
> > @@ -1,33 +0,0 @@
> > -Analog Devices ADXL372 3-Axis, +/-(200g) Digital Accelerometer
> > -
> > -http://www.analog.com/media/en/technical-documentation/data-sheets/adxl372.pdf
> > -
> > -Required properties:
> > - - compatible : should be "adi,adxl372"
> > - - reg: the I2C address or SPI chip select number for the device
> > -
> > -Required properties for SPI bus usage:
> > - - spi-max-frequency: Max SPI frequency to use
> > -
> > -Optional properties:
> > - - interrupts: interrupt mapping for IRQ as documented in
> > - Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> > -
> >
Re: [RESEND PATCH] staging: iio: ad7192: create of_device_id array
On Sun, May 19, 2019 at 8:53 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 19:44:35 -0300
> Bárbara Fernandes wrote:
>
I don't have anything else on top of what Jonathan added.
Acked-by: Alexandru Ardelean
CC-ing my work-email
There are some issues with it and mailing lists; I'll hopefully sort
them out in the next weeks.
> > Create list of compatible device ids to be matched with those stated in
> > the device tree.
> >
> > Signed-off-by: Bárbara Fernandes
> > Signed-off-by: Wilson Sales
> > Co-developed by: Wilson Sales
> Hi Bárbara, Wilson,
>
> One minor issue inline about code ordering.
> Actual content is fine.
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/staging/iio/adc/ad7192.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/staging/iio/adc/ad7192.c
> > b/drivers/staging/iio/adc/ad7192.c
> > index 3d74da9d37e7..cc886f944dbf 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -810,11 +810,23 @@ static const struct spi_device_id ad7192_id[] = {
> > {"ad7195", ID_AD7195},
> > {}
> > };
> > +
> > +static const struct of_device_id ad7192_of_spi_match[] = {
> > + { .compatible = "adi,ad7190" },
> > + { .compatible = "adi,ad7192" },
> > + { .compatible = "adi,ad7193" },
> > + { .compatible = "adi,ad7195" },
> > + {}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, ad7192_of_spi_match);
> > +
> Please keep the declaration of the table alongside the relevant
> MODULE_DEVICE_TABLE.
>
> In short, better to have your additions after this next line.
> > MODULE_DEVICE_TABLE(spi, ad7192_id);
> >
> > static struct spi_driver ad7192_driver = {
> > .driver = {
> > .name = "ad7192",
> > + .of_match_table = ad7192_of_spi_match,
> > },
> > .probe = ad7192_probe,
> > .remove = ad7192_remove,
>
Re: [PATCH] staging:iio:ad7150: fix threshold mode config bit
On Sun, May 19, 2019 at 8:38 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 22:04:56 -0300
> Melissa Wen wrote:
>
> > According to the AD7150 configuration register description, bit 7 assumes
> > value 1 when the threshold mode is fixed and 0 when it is adaptive,
> > however, the operation that identifies this mode was considering the
> > opposite values.
> >
> > This patch renames the boolean variable to describe it correctly and
> > properly replaces it in the places where it is used.
> >
> > Fixes: 531efd6aa0991 ("staging:iio:adc:ad7150: chan_spec conv + i2c_smbus
> > commands + drop unused poweroff timeout control.")
> > Signed-off-by: Melissa Wen
>
> Looks good to me. Applied to the fixes-togreg branch of iio.git pushed out as
> as testing-fixes for the autobuilders to see if they can find anything
> we have missed.
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/staging/iio/cdc/ad7150.c | 19 +++
> > 1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/iio/cdc/ad7150.c
> > b/drivers/staging/iio/cdc/ad7150.c
> > index dd7fcab8e19e..e075244c602b 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -5,6 +5,7 @@
> > * Copyright 2010-2011 Analog Devices Inc.
> > */
> >
> > +#include
> > #include
> > #include
> > #include
> > @@ -130,7 +131,7 @@ static int ad7150_read_event_config(struct iio_dev
> > *indio_dev,
> > {
> > int ret;
> > u8 threshtype;
> > - bool adaptive;
> > + bool thrfixed;
> > struct ad7150_chip_info *chip = iio_priv(indio_dev);
> >
> > ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> > @@ -138,21 +139,23 @@ static int ad7150_read_event_config(struct iio_dev
> > *indio_dev,
> > return ret;
> >
> > threshtype = (ret >> 5) & 0x03;
> > - adaptive = !!(ret & 0x80);
> > +
> > + /*check if threshold mode is fixed or adaptive*/
> > + thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
nitpick: i would have kept the original variable name as "adaptive",
mostly for consistency.
"adaptive" is used in other places as well;
as i recall, the fix is just oneliner in this case:
- adaptive = !!(ret & 0x80);
+ adaptive = !(ret & 0x80);
> >
> > switch (type) {
> > case IIO_EV_TYPE_MAG_ADAPTIVE:
> > if (dir == IIO_EV_DIR_RISING)
> > - return adaptive && (threshtype == 0x1);
> > - return adaptive && (threshtype == 0x0);
> > + return !thrfixed && (threshtype == 0x1);
> > + return !thrfixed && (threshtype == 0x0);
> > case IIO_EV_TYPE_THRESH_ADAPTIVE:
> > if (dir == IIO_EV_DIR_RISING)
> > - return adaptive && (threshtype == 0x3);
> > - return adaptive && (threshtype == 0x2);
> > + return !thrfixed && (threshtype == 0x3);
> > + return !thrfixed && (threshtype == 0x2);
> > case IIO_EV_TYPE_THRESH:
> > if (dir == IIO_EV_DIR_RISING)
> > - return !adaptive && (threshtype == 0x1);
> > - return !adaptive && (threshtype == 0x0);
> > + return thrfixed && (threshtype == 0x1);
> > + return thrfixed && (threshtype == 0x0);
> > default:
> > break;
> > }
>
Re: [RESEND PATCH] staging: iio: adt7316: create of_device_id array
On Sun, May 19, 2019 at 8:54 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 19:43:33 -0300
> Bárbara Fernandes wrote:
>
> > Create structure of type of_device_id in order to register all devices
> > the driver is able to manage.
> >
> > Signed-off-by: Bárbara Fernandes
> > Signed-off-by: Wilson Sales
> > Co-developed-by: Wilson Sales
> Looks good to me.
>
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
>
> Thanks,
Also,
Acked-by: Alexandru Ardelean
CC-ing my work-email
There are some issues with it and mailing lists; I'll hopefully sort
them out in the next weeks.
>
> Jonathan
>
> > ---
> > drivers/staging/iio/addac/adt7316-spi.c | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/staging/iio/addac/adt7316-spi.c
> > b/drivers/staging/iio/addac/adt7316-spi.c
> > index 8294b9c1e3c2..9968775f1d23 100644
> > --- a/drivers/staging/iio/addac/adt7316-spi.c
> > +++ b/drivers/staging/iio/addac/adt7316-spi.c
> > @@ -127,9 +127,22 @@ static const struct spi_device_id adt7316_spi_id[] = {
> >
> > MODULE_DEVICE_TABLE(spi, adt7316_spi_id);
> >
> > +static const struct of_device_id adt7316_of_spi_match[] = {
> > + { .compatible = "adi,adt7316" },
> > + { .compatible = "adi,adt7317" },
> > + { .compatible = "adi,adt7318" },
> > + { .compatible = "adi,adt7516" },
> > + { .compatible = "adi,adt7517" },
> > + { .compatible = "adi,adt7519" },
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, adt7316_of_spi_match);
> > +
> > static struct spi_driver adt7316_driver = {
> > .driver = {
> > .name = "adt7316",
> > + .of_match_table = adt7316_of_spi_match,
> > .pm = ADT7316_PM_OPS,
> > },
> > .probe = adt7316_spi_probe,
>
Re: [PATCH] staging: iio: ad9832: Add device tree support
On Sun, May 19, 2019 at 8:17 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 17:48:25 -0300
> João Seckler wrote:
>
> > Add a of_device_id struct variable and subsequent call to
> > MODULE_DEVICE_TABLE macro to support device tree.
> >
> > Signed-off-by: João Seckler
> > Signed-off-by: Anderson Reis
> > Co-developed-by: Anderson Reis
> > Signed-off-by: Andre Tadeu de Carvalho
> > Co-developed-by: Andre Tadeu de Carvalho
> Hi All,
>
> Missing the setting of the relevant entry in the spi_driver structure.
> Otherwise looks fine,
>
> Thanks,
>
> Jonathan
> > ---
> > drivers/staging/iio/frequency/ad9832.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/staging/iio/frequency/ad9832.c
> > b/drivers/staging/iio/frequency/ad9832.c
> > index 74308a2e72db..51e97c74c6b2 100644
> > --- a/drivers/staging/iio/frequency/ad9832.c
> > +++ b/drivers/staging/iio/frequency/ad9832.c
> > @@ -451,6 +451,13 @@ static int ad9832_remove(struct spi_device *spi)
> > return 0;
> > }
> >
> > +static const struct of_device_id ad9832_of_match[] = {
> > + { .compatible = "adi,ad9832", },
> > + { .compatible = "adi,ad9835", },
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, ad9832_of_match);
> > +
Yep.
To clarify what Jonathan said (see line below with plus + ) :
static struct spi_driver ad9832_driver = {
.driver = {
.name = "ad9832",
+ .of_match_table = ad9832_of_match,
},
.probe = ad9832_probe,
.remove = ad9832_remove,
.id_table = ad9832_id,
};
> > static const struct spi_device_id ad9832_id[] = {
> > {"ad9832", 0},
> > {"ad9835", 0},
>
[PATCH 03/16] lib,treewide: add new match_string() helper/macro
This change re-introduces `match_string()` as a macro that uses
ARRAY_SIZE() to compute the size of the array.
The macro is added in all the places that do
`match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
straightforward.
Signed-off-by: Alexandru Ardelean
---
drivers/clk/bcm/clk-bcm2835.c| 4 +---
drivers/gpio/gpiolib-of.c| 2 +-
drivers/gpu/drm/i915/intel_pipe_crc.c| 2 +-
drivers/mfd/omap-usb-host.c | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +-
drivers/pci/pcie/aer.c | 2 +-
drivers/usb/common/common.c | 4 ++--
drivers/usb/typec/class.c| 8 +++-
drivers/usb/typec/tps6598x.c | 2 +-
drivers/vfio/vfio.c | 4 +---
include/linux/string.h | 9 +
sound/firewire/oxfw/oxfw.c | 2 +-
sound/soc/codecs/max98088.c | 2 +-
sound/soc/codecs/max98095.c | 2 +-
14 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index a775f6a1f717..1ab388590ead 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1390,9 +1390,7 @@ static struct clk_hw *bcm2835_register_clock(struct
bcm2835_cprman *cprman,
for (i = 0; i < data->num_mux_parents; i++) {
parents[i] = data->parents[i];
- ret = __match_string(cprman_parent_names,
-ARRAY_SIZE(cprman_parent_names),
-parents[i]);
+ ret = match_string(cprman_parent_names, parents[i]);
if (ret >= 0)
parents[i] = cprman->real_parent_names[ret];
}
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 27d6f04ab58e..71e886869d78 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -279,7 +279,7 @@ static struct gpio_desc *of_find_regulator_gpio(struct
device *dev, const char *
if (!con_id)
return ERR_PTR(-ENOENT);
- i = __match_string(whitelist, ARRAY_SIZE(whitelist), con_id);
+ i = match_string(whitelist, con_id);
if (i < 0)
return ERR_PTR(-ENOENT);
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c
b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 286fad1f0e08..6fc4f3d3d1f6 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -449,7 +449,7 @@ display_crc_ctl_parse_source(const char *buf, enum
intel_pipe_crc_source *s)
return 0;
}
- i = __match_string(pipe_crc_sources, ARRAY_SIZE(pipe_crc_sources), buf);
+ i = match_string(pipe_crc_sources, buf);
if (i < 0)
return i;
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 9aaacb5bdb26..53dff34c0afc 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -509,7 +509,7 @@ static int usbhs_omap_get_dt_pdata(struct device *dev,
continue;
/* get 'enum usbhs_omap_port_mode' from port mode string */
- ret = __match_string(port_modes, ARRAY_SIZE(port_modes), mode);
+ ret = match_string(port_modes, mode);
if (ret < 0) {
dev_warn(dev, "Invalid port%d-mode \"%s\" in device
tree\n",
i, mode);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index 59ce3ff35553..778b4dfd8b75 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -667,7 +667,7 @@ iwl_dbgfs_bt_force_ant_write(struct iwl_mvm *mvm, char *buf,
};
int ret, bt_force_ant_mode;
- ret = __match_string(modes_str, ARRAY_SIZE(modes_str), buf);
+ ret = match_string(modes_str, buf);
if (ret < 0)
return ret;
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 41a0773a1cbc..2278caba109c 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -203,7 +203,7 @@ void pcie_ecrc_get_policy(char *str)
{
int i;
- i = __match_string(ecrc_policy_str, ARRAY_SIZE(ecrc_policy_str), str);
+ i = match_string(ecrc_policy_str, str);
if (i < 0)
return;
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index bca0c404c6ca..5a651d311d38 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -68,7 +68,7 @@ enum usb_device_speed usb_get_maximum_speed(struct device
*dev)
if (ret < 0)
return USB_SPEED_UNKNOWN;
- ret = __match_string(spe
[PATCH 06/16] x86/mtrr: use new match_string() helper + add gaps == minor fix
This change is a bit more than cosmetic.
It replaces 2 values in mtrr_strings with NULL. Previously, they were
defined as "?", which is not great because you could technically pass "?",
and you would get value 2.
It's not sure whether that was intended (likely it wasn't), but this fixes
that.
Signed-off-by: Alexandru Ardelean
---
arch/x86/kernel/cpu/mtrr/if.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index 4ec7a5f7b94c..e67820a044cc 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -20,8 +20,8 @@ static const char *const mtrr_strings[MTRR_NUM_TYPES] =
{
"uncachable", /* 0 */
"write-combining", /* 1 */
- "?",/* 2 */
- "?",/* 3 */
+ NULL, /* 2 */
+ NULL, /* 3 */
"write-through",/* 4 */
"write-protect",/* 5 */
"write-back", /* 6 */
@@ -29,7 +29,9 @@ static const char *const mtrr_strings[MTRR_NUM_TYPES] =
const char *mtrr_attrib_to_str(int x)
{
- return (x <= 6) ? mtrr_strings[x] : "?";
+ if ((x >= ARRAY_SIZE(mtrr_strings)) || (mtrr_strings[x] == NULL))
+ return "?";
+ return mtrr_strings[x];
}
#ifdef CONFIG_PROC_FS
@@ -142,7 +144,7 @@ mtrr_write(struct file *file, const char __user *buf,
size_t len, loff_t * ppos)
return -EINVAL;
ptr = skip_spaces(ptr + 5);
- i = __match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
+ i = match_string(mtrr_strings, ptr);
if (i < 0)
return i;
--
2.17.1
[PATCH 10/16] pinctrl: armada-37xx: use new match_string() helper/macro
The change is mostly cosmetic.
The `armada_37xx_pin_group` struct is defined as.
struct armada_37xx_pin_group {
const char *name;
unsigned intstart_pin;
unsigned intnpins;
u32 reg_mask;
u32 val[NB_FUNCS];
unsigned intextra_pin;
unsigned intextra_npins;
const char *funcs[NB_FUNCS];
unsigned int*pins;
};
The `funcs` field is a static array of strings, so using the
new `match_string()` helper (which does an implicit ARRAY_SIZE(gp->funcs))
should be fine.
Signed-off-by: Alexandru Ardelean
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 07a5bcaa0067..68b0db5ef5e9 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -348,7 +348,7 @@ static int armada_37xx_pmx_set_by_name(struct pinctrl_dev
*pctldev,
dev_dbg(info->dev, "enable function %s group %s\n",
name, grp->name);
- func = __match_string(grp->funcs, NB_FUNCS, name);
+ func = match_string(grp->funcs, name);
if (func < 0)
return -ENOTSUPP;
@@ -938,7 +938,7 @@ static int armada_37xx_fill_func(struct armada_37xx_pinctrl
*info)
struct armada_37xx_pin_group *gp = &info->groups[g];
int f;
- f = __match_string(gp->funcs, NB_FUNCS, name);
+ f = match_string(gp->funcs, name);
if (f < 0)
continue;
--
2.17.1
[PATCH 15/16] video: fbdev: pxafb: use new match_string() helper/macro
The `lcd_types` array is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.
This reduces the array by 1 element, since the NULL (at the end of the
array) is no longer needed.
Signed-off-by: Alexandru Ardelean
---
drivers/video/fbdev/pxafb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 0025781e6e1e..e657a04f5b1d 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -2114,7 +2114,7 @@ static void pxafb_check_options(struct device *dev,
struct pxafb_mach_info *inf)
#if defined(CONFIG_OF)
static const char * const lcd_types[] = {
"unknown", "mono-stn", "mono-dstn", "color-stn", "color-dstn",
- "color-tft", "smart-panel", NULL
+ "color-tft", "smart-panel"
};
static int of_get_pxafb_display(struct device *dev, struct device_node *disp,
@@ -2129,7 +2129,7 @@ static int of_get_pxafb_display(struct device *dev,
struct device_node *disp,
if (ret)
s = "color-tft";
- i = __match_string(lcd_types, -1, s);
+ i = match_string(lcd_types, s);
if (i < 0) {
dev_err(dev, "lcd-type %s is unknown\n", s);
return i;
--
2.17.1
[PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro
The change is also cosmetic, but it also does a tighter coupling between
the enums & the string values. This way, the ARRAY_SIZE(phy_types) that is
implicitly done in the match_string() macro is also a bit safer.
Signed-off-by: Alexandru Ardelean
---
drivers/mmc/host/sdhci-xenon-phy.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sdhci-xenon-phy.c
b/drivers/mmc/host/sdhci-xenon-phy.c
index 59b7a6cac995..2a9206867fe1 100644
--- a/drivers/mmc/host/sdhci-xenon-phy.c
+++ b/drivers/mmc/host/sdhci-xenon-phy.c
@@ -135,17 +135,17 @@ struct xenon_emmc_phy_regs {
u32 logic_timing_val;
};
-static const char * const phy_types[] = {
- "emmc 5.0 phy",
- "emmc 5.1 phy"
-};
-
enum xenon_phy_type_enum {
EMMC_5_0_PHY,
EMMC_5_1_PHY,
NR_PHY_TYPES
};
+static const char * const phy_types[NR_PHY_TYPES] = {
+ [EMMC_5_0_PHY] = "emmc 5.0 phy",
+ [EMMC_5_1_PHY] = "emmc 5.1 phy"
+};
+
enum soc_pad_ctrl_type {
SOC_PAD_SD,
SOC_PAD_FIXED_1_8V,
@@ -821,7 +821,7 @@ static int xenon_add_phy(struct device_node *np, struct
sdhci_host *host,
struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
int ret;
- priv->phy_type = __match_string(phy_types, NR_PHY_TYPES, phy_name);
+ priv->phy_type = match_string(phy_types, phy_name);
if (priv->phy_type < 0) {
dev_err(mmc_dev(host->mmc),
"Unable to determine PHY name %s. Use default eMMC 5.1
PHY\n",
--
2.17.1
[PATCH 05/16] ALSA: oxygen: use new match_string() helper/macro
The change is purely cosmetic at this point in time, but it does highlight
the change done in lib/string.c for match_string().
Particularly for this change, a control mode can be removed/added at a
different index/enum-value, and the match_string() helper will continue
until the end of the array and ignore the NULL.
Signed-off-by: Alexandru Ardelean
---
sound/pci/oxygen/oxygen_mixer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/oxygen/oxygen_mixer.c b/sound/pci/oxygen/oxygen_mixer.c
index 13c2fb75fd71..961fd1cbc712 100644
--- a/sound/pci/oxygen/oxygen_mixer.c
+++ b/sound/pci/oxygen/oxygen_mixer.c
@@ -1086,7 +1086,7 @@ static int add_controls(struct oxygen *chip,
err = snd_ctl_add(chip->card, ctl);
if (err < 0)
return err;
- j = __match_string(known_ctl_names, CONTROL_COUNT,
ctl->id.name);
+ j = match_string(known_ctl_names, ctl->id.name);
if (j >= 0) {
chip->controls[j] = ctl;
ctl->private_free = oxygen_any_ctl_free;
--
2.17.1
[PATCH 00/16] treewide: fix match_string() helper when array size
The intent of this patch series is to make a case for fixing the match_string() string helper. The doc-string of the `__sysfs_match_string()` helper mentions that `n` (the size of the given array) should be: * @n: number of strings in the array or -1 for NULL terminated arrays However, this is not the case. The helper stops on the first NULL in the array, regardless of whether -1 is provided or not. There are some advantages to allowing this behavior (NULL elements within in the array). One example, is to allow reserved registers as NULL in an array. One example in the series is patch: x86/mtrr: use new match_string() helper + add gaps == minor fix which uses a "?" string for values that are reserved/don't care. Since the change is a bit big, the change was coupled with renaming match_string() -> __match_string(). The new match_string() helper (resulted here) does an ARRAY_SIZE() over the array, which is useful when the array is static. Also, this way of doing things is a way to go through all the users of this helpers and check that nothing goes wrong, and notify them about the change to match_string(). It's a way of grouping changes in a manage-able way. The first patch is important, the others can be dropped. Signed-off-by: Alexandru Ardelean Alexandru Ardelean (16): lib: fix match_string() helper when array size is positive treewide: rename match_string() -> __match_string() lib,treewide: add new match_string() helper/macro powerpc/xmon: use new match_string() helper/macro ALSA: oxygen: use new match_string() helper/macro x86/mtrr: use new match_string() helper + add gaps == minor fix device connection: use new match_string() helper/macro cpufreq/intel_pstate: remove NULL entry + use match_string() mmc: sdhci-xenon: use new match_string() helper/macro pinctrl: armada-37xx: use new match_string() helper/macro mm/vmpressure.c: use new match_string() helper/macro rdmacg: use new match_string() helper/macro drm/edid: use new match_string() helper/macro staging: gdm724x: use new match_string() helper/macro video: fbdev: pxafb: use new match_string() helper/macro sched: debug: use new match_string() helper/macro arch/powerpc/xmon/xmon.c | 2 +- arch/x86/kernel/cpu/mtrr/if.c| 10 ++ drivers/ata/pata_hpt366.c| 2 +- drivers/ata/pata_hpt37x.c| 2 +- drivers/base/devcon.c| 2 +- drivers/base/property.c | 2 +- drivers/clk/bcm/clk-bcm2835.c| 4 +--- drivers/clk/clk.c| 4 ++-- drivers/clk/rockchip/clk.c | 4 ++-- drivers/cpufreq/intel_pstate.c | 9 - drivers/gpio/gpiolib-of.c| 2 +- drivers/gpu/drm/drm_edid_load.c | 2 +- drivers/gpu/drm/drm_panel_orientation_quirks.c | 2 +- drivers/gpu/drm/i915/intel_pipe_crc.c| 2 +- drivers/ide/hpt366.c | 2 +- drivers/mfd/omap-usb-host.c | 2 +- drivers/mmc/host/sdhci-xenon-phy.c | 12 ++-- drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +- drivers/pci/pcie/aer.c | 2 +- drivers/phy/tegra/xusb.c | 2 +- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++-- drivers/pinctrl/pinmux.c | 2 +- drivers/power/supply/ab8500_btemp.c | 2 +- drivers/power/supply/ab8500_charger.c| 2 +- drivers/power/supply/ab8500_fg.c | 2 +- drivers/power/supply/abx500_chargalg.c | 2 +- drivers/power/supply/charger-manager.c | 4 ++-- drivers/staging/gdm724x/gdm_tty.c| 3 +-- drivers/usb/common/common.c | 4 ++-- drivers/usb/typec/class.c| 8 +++- drivers/usb/typec/tps6598x.c | 2 +- drivers/vfio/vfio.c | 4 +--- drivers/video/fbdev/pxafb.c | 4 ++-- fs/ubifs/auth.c | 4 ++-- include/linux/string.h | 11 ++- kernel/cgroup/rdma.c | 2 +- kernel/sched/debug.c | 2 +- kernel/trace/trace.c | 2 +- lib/string.c | 13 - mm/mempolicy.c | 2 +- mm/vmpressure.c | 4 ++-- security/apparmor/lsm.c | 4 ++-- security/integrity/ima/ima_main.c| 2 +- sound/firewire/oxfw/oxfw.c | 2 +- sound/pci/oxygen/oxygen_mixer.c | 2 +- sound/soc/codecs/max98088.c | 2 +- sound/soc/codecs/max98095.c
[PATCH 12/16] rdmacg: use new match_string() helper/macro
The `rdmacg_resource_names` array is a static array of strings. Using match_string() (which computes the array size via ARRAY_SIZE()) is possible. The change is mostly cosmetic. No functionality change. Signed-off-by: Alexandru Ardelean --- kernel/cgroup/rdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/cgroup/rdma.c b/kernel/cgroup/rdma.c index 65d4df148603..71c3d305bd1f 100644 --- a/kernel/cgroup/rdma.c +++ b/kernel/cgroup/rdma.c @@ -367,7 +367,7 @@ static int parse_resource(char *c, int *intval) if (!name || !value) return -EINVAL; - i = __match_string(rdmacg_resource_names, RDMACG_RESOURCE_MAX, name); + i = match_string(rdmacg_resource_names, name); if (i < 0) return i; -- 2.17.1
[PATCH 2/3][V3] scsi: sd: remove sysfs_match_string() dense array comment
The comment is no longer valid, since it supports arrays with gaps now.
Signed-off-by: Alexandru Ardelean
---
Changelog v2 -> v3:
* after fixing __sysfs_match_string() this comment is no longer valid
drivers/scsi/sd.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 2b2bc4b49d78..73ce390956c1 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -376,7 +376,6 @@ thin_provisioning_show(struct device *dev, struct
device_attribute *attr,
}
static DEVICE_ATTR_RO(thin_provisioning);
-/* sysfs_match_string() requires dense arrays */
static const char *lbp_mode[] = {
[SD_LBP_FULL] = "full",
[SD_LBP_UNMAP] = "unmap",
@@ -424,7 +423,6 @@ provisioning_mode_store(struct device *dev, struct
device_attribute *attr,
}
static DEVICE_ATTR_RW(provisioning_mode);
-/* sysfs_match_string() requires dense arrays */
static const char *zeroing_mode[] = {
[SD_ZERO_WRITE] = "write",
[SD_ZERO_WS]= "writesame",
--
2.17.1
Re: [PATCH 1/2] lib: add __sysfs_match_string_with_gaps() helper
On Fri, Apr 26, 2019 at 5:27 PM andriy.shevche...@linux.intel.com wrote: > > On Fri, Apr 26, 2019 at 12:29:11PM +0300, Alexandru Ardelean wrote: > > > Hmm, I actually did not give much thought to that -1. > > I'll check into this and see about a V3. > > It may make more sense to just fix the original > > `__sysfs_match_string()`, but I'll need to go through the users of > > this function and see. > > I was thinking about existing users of such (with "gaps") cases. > Not all of them have NULL there and would like to avoid some members. > Though, I think that we may ignore NULL items if -1 is supplied. > > Think as well about ARRAY_SIZE() as given to that. > I am a bit vague on what you are proposing. Is it: a) Leave __sysfs_match_string() as-is and introduce a new `__sysfs_match_string_with_gaps()` helper/variant ? b) Fix __sysfs_match_string() to break/exit on the first NULL, only if -1 is provided ? Either is fine, but I wanted to clarify. Thanks Alex > And consider to fix match_string() accordingly. > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH 0/4] staging: iio: ad7150: improve driver readability
On Sat, May 4, 2019 at 2:12 PM Alexandru Ardelean wrote: > > On Sat, May 4, 2019 at 1:24 AM Melissa Wen wrote: > > > > This patchset solves readability issues in AD7150 code, such as clarify > > register and mask definition, fashion improvement of mask uses, reduce > > tedious operation and useless comments. > > > > Hey, > > Two patches seem a bit noisy/un-needed. > The other 2 are fine from me. > > This driver does need some work to move it out of staging. > I am not sure what would be a big blocker for it, other than maybe it > needs a device-tree binding doc (in YAML format). > Maybe Jonathan remembers. > > Some other low-hanging-fruit ideas would be: > 1) remove the code for platform_data ; that one seems forgotten from > some other time; the interrupts should be coming from device-tree, > from the i2c bindings > 2) you could do a AD7150_EVENT_SPEC() macro (similar to > AD7150_TIMEOUT() macro) and use it in the ad7150_events[] list; that > would reduce a few lines > 3) similar to 2), you could do a AD7150_CHANNEL(x) macro ; > 4) in ad7150_event_handler() the checks could be wrapped into a macro, > or maybe some function ; i am referring to "(int_status & > AD7150_STATUS_OUT1) && (chip->old_state & AD7150_STATUS_OUT1)" checks > ; those seem to be repeated > 5) add of_match_table to the driver > > I (now) suspect that the reason this driver is still in staging is this > comment: > /* Timeouts not currently handled by core */ > > I wonder if things changed since then ? > If not, it would be interesting to implement it in core. > I forgot to mention the wiki page for the driver: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-cdc/ad7150 it may help with a few things > Thanks > Alex > > > > Melissa Wen (4): > > staging: iio: ad7150: organize registers definition > > staging: iio: ad7150: use FIELD_GET and GENMASK > > staging: iio: ad7150: simplify i2c SMBus return treatment > > staging: iio: ad7150: clean up of comments > > > > drivers/staging/iio/cdc/ad7150.c | 102 ++- > > 1 file changed, 47 insertions(+), 55 deletions(-) > > > > -- > > 2.20.1 > >
Re: [PATCH 0/4] staging: iio: ad7150: improve driver readability
On Sat, May 4, 2019 at 1:24 AM Melissa Wen wrote: > > This patchset solves readability issues in AD7150 code, such as clarify > register and mask definition, fashion improvement of mask uses, reduce > tedious operation and useless comments. > Hey, Two patches seem a bit noisy/un-needed. The other 2 are fine from me. This driver does need some work to move it out of staging. I am not sure what would be a big blocker for it, other than maybe it needs a device-tree binding doc (in YAML format). Maybe Jonathan remembers. Some other low-hanging-fruit ideas would be: 1) remove the code for platform_data ; that one seems forgotten from some other time; the interrupts should be coming from device-tree, from the i2c bindings 2) you could do a AD7150_EVENT_SPEC() macro (similar to AD7150_TIMEOUT() macro) and use it in the ad7150_events[] list; that would reduce a few lines 3) similar to 2), you could do a AD7150_CHANNEL(x) macro ; 4) in ad7150_event_handler() the checks could be wrapped into a macro, or maybe some function ; i am referring to "(int_status & AD7150_STATUS_OUT1) && (chip->old_state & AD7150_STATUS_OUT1)" checks ; those seem to be repeated 5) add of_match_table to the driver I (now) suspect that the reason this driver is still in staging is this comment: /* Timeouts not currently handled by core */ I wonder if things changed since then ? If not, it would be interesting to implement it in core. Thanks Alex > Melissa Wen (4): > staging: iio: ad7150: organize registers definition > staging: iio: ad7150: use FIELD_GET and GENMASK > staging: iio: ad7150: simplify i2c SMBus return treatment > staging: iio: ad7150: clean up of comments > > drivers/staging/iio/cdc/ad7150.c | 102 ++- > 1 file changed, 47 insertions(+), 55 deletions(-) > > -- > 2.20.1 >
Re: [PATCH 2/4] staging: iio: ad7150: use FIELD_GET and GENMASK
On Sat, May 4, 2019 at 1:25 AM Melissa Wen wrote:
>
> Use the bitfield macro FIELD_GET, and GENMASK to do the shift and mask in
> one go. This makes the code more readable than explicit masking followed
> by a shift.
>
This looks neat.
I'd have to remember to ack it from my work email.
One minor comment inline, which would be the object of a new patch.
> Signed-off-by: Melissa Wen
> ---
> drivers/staging/iio/cdc/ad7150.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index 24601ba7db88..4ba46fb6ac02 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -5,6 +5,7 @@
> * Copyright 2010-2011 Analog Devices Inc.
> */
>
> +#include
> #include
> #include
> #include
> @@ -44,6 +45,9 @@
> #define AD7150_SN0_REG 0x16
> #define AD7150_ID_REG 0x17
>
> +/* AD7150 masks */
> +#define AD7150_THRESHTYPE_MSK GENMASK(6, 5)
> +
> /**
> * struct ad7150_chip_info - instance specific chip data
> * @client: i2c client for this device
> @@ -136,7 +140,7 @@ static int ad7150_read_event_config(struct iio_dev
> *indio_dev,
> if (ret < 0)
> return ret;
>
> - threshtype = (ret >> 5) & 0x03;
> + threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
> adaptive = !!(ret & 0x80);
Why not also do something similar for the `adaptive` value ?
>
> switch (type) {
> --
> 2.20.1
>
Re: [PATCH 3/4] staging: iio: ad7150: simplify i2c SMBus return treatment
On Sat, May 4, 2019 at 1:26 AM Melissa Wen wrote: > > Since i2c_smbus_write_byte_data returns no-positive value, this commit > making the treatment of its return value less verbose. > > Signed-off-by: Melissa Wen > --- > drivers/staging/iio/cdc/ad7150.c | 10 +++--- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7150.c > b/drivers/staging/iio/cdc/ad7150.c > index 4ba46fb6ac02..3a4572a9e5ec 100644 > --- a/drivers/staging/iio/cdc/ad7150.c > +++ b/drivers/staging/iio/cdc/ad7150.c > @@ -201,16 +201,12 @@ static int ad7150_write_event_params(struct iio_dev > *indio_dev, > ret = i2c_smbus_write_byte_data(chip->client, > ad7150_addresses[chan][4], > sens); > - if (ret < 0) > + if (ret) For i2c_smbus_write_byte_data(), checking "ret < 0" or non-zero, is the same. Changing this doesn't have any added value. > return ret; > - > - ret = i2c_smbus_write_byte_data(chip->client, > + else > + return i2c_smbus_write_byte_data(chip->client, > ad7150_addresses[chan][5], > timeout); The introduction of the "else" branch is a bit noisy. The code was a bit neater (and readable) before the else branch, and functionally identical. Well, when I say neater before, you have to understand, that I (and I assume that some other people who write drivers) have a slight fixation for this pattern: example1: ret = fn1(); if (ret < 0) // could also be just "if (ret)" return ret; ret = fn2(); if (ret < 0) // could also be just "if (ret)" return ret; example1a: +ret = fn3(); +if (ret < 0) // could also be just "if (ret)" +return ret; Various higher-level programming languages, will discourage this pattern in favor of neater patterns. I personally, have a few arguments in favor of this pattern: 1) it is closer to how the machine code ; so, closer to how a low-level instruction looks like 2) if (ever) this needs to be patched, the patch could be neat (see example1a) ; the examle assumes that it's been added via a patch at a later point in time 3) it keeps indentation level to a minimum ; this also aligns with kernel-coding guidelines (https://www.kernel.org/doc/html/v4.10/process/coding-style.html ) (indentation seems a bit OCD-like when someone points it out at a review, but it has it's value over time) > - if (ret < 0) > - return ret; > - > - return 0; > } > > static int ad7150_write_event_config(struct iio_dev *indio_dev, > -- > 2.20.1 >
Re: [PATCH 1/4] staging: iio: ad7150: organize registers definition
On Sat, May 4, 2019 at 1:25 AM Melissa Wen wrote:
>
> Use the suffix REG to make the register addresses clear
> and indentation to highlight field names.
>
I'm inclined to say that this change is a bit too much noise versus added value.
While the REG suffix does make sense (generally), since it hasn't been
added from the beginning, it doesn't make much sense to add it now.
It is sufficiently clear (as-is) that these macros refer to registers.
They should be easy to match with the datasheet as well.
> Signed-off-by: Melissa Wen
> ---
> drivers/staging/iio/cdc/ad7150.c | 75
> 1 file changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index dd7fcab8e19e..24601ba7db88 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -15,35 +15,34 @@
> #include
> #include
> #include
> -/*
> - * AD7150 registers definition
> - */
>
> -#define AD7150_STATUS 0
> -#define AD7150_STATUS_OUT1 BIT(3)
> -#define AD7150_STATUS_OUT2 BIT(5)
> -#define AD7150_CH1_DATA_HIGH 1
> -#define AD7150_CH2_DATA_HIGH 3
> -#define AD7150_CH1_AVG_HIGH5
> -#define AD7150_CH2_AVG_HIGH7
> -#define AD7150_CH1_SENSITIVITY 9
> -#define AD7150_CH1_THR_HOLD_H 9
> -#define AD7150_CH1_TIMEOUT 10
> -#define AD7150_CH1_SETUP 11
> -#define AD7150_CH2_SENSITIVITY 12
> -#define AD7150_CH2_THR_HOLD_H 12
> -#define AD7150_CH2_TIMEOUT 13
> -#define AD7150_CH2_SETUP 14
> -#define AD7150_CFG 15
> -#define AD7150_CFG_FIX BIT(7)
> -#define AD7150_PD_TIMER16
> -#define AD7150_CH1_CAPDAC 17
> -#define AD7150_CH2_CAPDAC 18
> -#define AD7150_SN3 19
> -#define AD7150_SN2 20
> -#define AD7150_SN1 21
> -#define AD7150_SN0 22
> -#define AD7150_ID 23
> +/* AD7150 registers */
> +
> +#define AD7150_STATUS_REG 0x00
> +#define AD7150_STATUS_OUT1 BIT(3)
> +#define AD7150_STATUS_OUT2 BIT(5)
> +#define AD7150_CH1_DATA_HIGH_REG 0x01
> +#define AD7150_CH2_DATA_HIGH_REG 0x03
> +#define AD7150_CH1_AVG_HIGH_REG0x05
> +#define AD7150_CH2_AVG_HIGH_REG0x07
> +#define AD7150_CH1_SENSITIVITY_REG 0x09
> +#define AD7150_CH1_THR_HOLD_H_REG 0x09
> +#define AD7150_CH2_SENSITIVITY_REG 0x0C
> +#define AD7150_CH1_TIMEOUT_REG 0x0A
> +#define AD7150_CH1_SETUP_REG 0x0B
> +#define AD7150_CH2_THR_HOLD_H_REG 0x0C
> +#define AD7150_CH2_TIMEOUT_REG 0x0D
> +#define AD7150_CH2_SETUP_REG 0x0E
> +#define AD7150_CFG_REG 0x0F
> +#define AD7150_CFG_FIX BIT(7)
> +#define AD7150_PD_TIMER_REG0x10
> +#define AD7150_CH1_CAPDAC_REG 0x11
> +#define AD7150_CH2_CAPDAC_REG 0x12
> +#define AD7150_SN3_REG 0x13
> +#define AD7150_SN2_REG 0x14
> +#define AD7150_SN1_REG 0x15
> +#define AD7150_SN0_REG 0x16
> +#define AD7150_ID_REG 0x17
>
> /**
> * struct ad7150_chip_info - instance specific chip data
> @@ -85,12 +84,12 @@ struct ad7150_chip_info {
> */
>
> static const u8 ad7150_addresses[][6] = {
> - { AD7150_CH1_DATA_HIGH, AD7150_CH1_AVG_HIGH,
> - AD7150_CH1_SETUP, AD7150_CH1_THR_HOLD_H,
> - AD7150_CH1_SENSITIVITY, AD7150_CH1_TIMEOUT },
> - { AD7150_CH2_DATA_HIGH, AD7150_CH2_AVG_HIGH,
> - AD7150_CH2_SETUP, AD7150_CH2_THR_HOLD_H,
> - AD7150_CH2_SENSITIVITY, AD7150_CH2_TIMEOUT },
> + { AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
> + AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
> + AD7150_CH1_SENSITIVITY_REG, AD7150_CH1_TIMEOUT_REG },
> + { AD7150_CH2_DATA_HIGH_REG, AD7150_CH2_AVG_HIGH_REG,
> + AD7150_CH2_SETUP_REG, AD7150_CH2_THR_HOLD_H_REG,
> + AD7150_CH2_SENSITIVITY_REG, AD7150_CH2_TIMEOUT_REG },
> };
>
> static int ad7150_read_raw(struct iio_dev *indio_dev,
> @@ -133,7 +132,7 @@ static int ad7150_read_event_config(struct iio_dev
> *indio_dev,
> bool adaptive;
> struct ad7150_chip_info *chip = iio_priv(indio_dev);
>
> - ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> + ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
> if (ret < 0)
> return ret;
>
> @@ -229,7 +228,7 @@ static int ad7150_write_event_config(struct iio_dev
> *indio_dev,
> if (event_code == chip->current_event)
> return 0;
> mutex_lock(&chip->state_lock
Re: [PATCH 1/2] lib: add __sysfs_match_string_with_gaps() helper
On Thu, Apr 25, 2019 at 10:38 PM gre...@linuxfoundation.org
wrote:
>
> On Wed, Apr 24, 2019 at 01:34:55PM +0100, Jonathan Cameron wrote:
> > On Tue, 23 Apr 2019 06:38:44 +
> > "Ardelean, Alexandru" wrote:
> >
> > > On Mon, 2019-04-22 at 23:06 +0200, Greg KH wrote:
> > > >
> > > >
> > > > On Mon, Apr 22, 2019 at 11:32:56AM +0300, Alexandru Ardelean wrote:
> > > > > This helper is similar to __sysfs_match_string() with the exception
> > > > > that it
> > > > > ignores NULL elements within the array.
> > > >
> > > > sysfs is "one value per file", why are you trying to write multiple
> > > > things on a single line to a single sysfs file?
> > > >
> > > > Is IIO really that messy? :)
> > > >
> > >
> > > Hmm, I don't think I understood the comment/question, or maybe I did not
> > > formulate the comment properly.
> > >
> > > Maybe Jonathan can pitch-in here if I'm saying something wrong.
> > >
> > > So, in IIO there is `struct iio_enum` which is essentially a sysfs wrapper
> > > for exposing an "enum" type to userspace via sysfs (which takes only one
> > > value). This iio_enum type is basically a string-to-int mapping.
> >
> > >
> > > Some example in C:
> > >
> > > enum {
> > > ENUM0,
> > > ENUM1,
> > > ENUM5 = 5,
> > > ENUM6,
> > > ENUM7
> > > };
> > >
> > >
> > > /* Notice the gaps in the elements */
> > > static const char * const item_strings[] = {
> > > [ENUM0] = "mode0",
> > > [ENUM1] = "mode1",
> > > [ENUM5] = "mode5",
> > > [ENUM6] = "mode6",
> > > [ENUM7] = "mode7",
> > > };
> > >
> > > static const struct iio_enum iio_enum1 = {
> > > .items = item_strings,
> > > .num_items = ARRAY_SIZE(item_strings),
> > > .set = iio_enum1_set,
> > > .get = iio_enum1_get,
> > > };
> > >
> > >
> > > The signature of the iio_enum1_set / iio_enum1_get is below:
> > >
> > > static int iio_enum1_set(struct iio_dev *indio_dev,
> > > const struct iio_chan_spec *chan, unsigned int val);
> > >
> > > static int iio_enum1_get(struct iio_dev *indio_dev,
> > > const struct iio_chan_spec *chan)
> > >
> > >
> > > IIO core resolves the string-to-int mapping.
> > > It uses __sysfs_match_string() to do that, but it requires that the list
> > > of
> > > strings (and C enums) be contiguous.
> > > This change [and V2 of this patch] introduces a
> > > __sysfs_match_string_with_gaps() helper that ignores gaps (represented as
> > > NULLs).
> > >
> > > For reference, __sysfs_match_string() returns -EINVAL on the first NULL in
> > > the array of strings (regardless of the given array size).
> > >
> > > __sysfs_match_string_with_gaps() is typically helpful when C enums refer
> > > to
> > > bitfields, or have some equivalence in HW.
> > >
> >
> > You have described it well.
> > Perhaps the issue is in the naming? Or more description is needed for the
> > original
> > patch.
> >
> > It's worth highlighting that the current help text for
> > __sysfs_match_string has a description that says:
> >
> > /**
> > * __sysfs_match_string - matches given string in an array
> > * @array: array of strings
> > * @n: number of strings in the array or -1 for NULL terminated arrays
> > * @str: string to match with
> > *
> > * Returns index of @str in the @array or -EINVAL, just like match_string().
> > * Uses sysfs_streq instead of strcmp for matching.
> > */
> >
> > so one could argue that if you pass a value of n which is not -1 the
> > function
> > should not assume that any NULL terminates the array...
> >
> > So basically this new function is implementing what I would have assumed
> > __sysfs_match_string would do, but doesn't.
>
> Ok, yeah, I'm confused, I also thought this is what the original
> function did.
>
> Nevermind, no objection from me on this:
>
> Acked-by: Greg Kroah-Hartman
Hmm, I actually did not give much thought to that -1.
I'll check into this and see about a V3.
It may make more sense to just fix the original
`__sysfs_match_string()`, but I'll need to go through the users of
this function and see.
Thanks
Alex
Re: [PATCH v2] iio: adc: ad7766: Change alignment to match paranthesis
On Wed, Apr 24, 2019 at 9:25 PM Camylla Cantanheide
wrote:
>
> Hello Alexandru,
>
> If you observe well the commit does the same kind of fix for the same file.
> In this case, the alignment of parenthesis in various parts of the code
> according to the checkpatch.pl's message.
>
> I believe, that he was raised correctly, only the description is detailed.
> The commit earlier, had been approved, but did not contain the S-o-B.
> However, if you think something should be changed and what should I change,
> it won't be a problem. Thanks!
>
Hey,
So, the title of the patch is:
"iio: adc: ad7766: Change alignment to match paranthesis"
However, the patch is changing more files:
.../bindings/iio/adc/nuvoton,npcm-adc.txt | 11 -
.../iio/chemical/plantower,pms7003.txt| 20 ++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/iio/adc/ad7766.c | 22 +-
drivers/iio/chemical/Kconfig | 10 +
drivers/iio/chemical/Makefile | 1 +
drivers/iio/chemical/pms7003.c| 340 ++
drivers/iio/chemical/sps30.c | 3 +
For one part, you are also adding a driver (pms7003.c).
The idea is that the title of the patch should summarize what the
patch is doing.
In this case, it's not clear.
It looks to me that the patch was generated incorrectly via a git command.
So, maybe try to regenerate the patch with just the ad7766 driver changes.
Thanks
Alex
> Best regards,
> Camylla Cantanheide
>
> Em qua, 24 de abr de 2019 às 04:43, Alexandru Ardelean
> escreveu:
>>
>> On Tue, Apr 23, 2019 at 11:04 PM Camylla Gonçalves Cantanheide
>> wrote:
>> >
>> > This commit align broken line to match upper line parenthesis,
>> > in lines 80, 130, 237, 242, 255, 264 and 293. Solves the checkpatch.pl's
>> > message:
>> >
>> > CHECK: Alignment should match open parenthesis
>> >
>> >In lines 130, 255, 264 and 293 it was necessary to break a line.
>> >
>>
>> This patch is errounous; seems it wasn't generated correctly.
>> It does too many things in one commit.
>>
>> > Signed-off-by: Camylla Gonçalves Cantanheide
>> > ---
>> > .../bindings/iio/adc/nuvoton,npcm-adc.txt | 11 -
>> > .../iio/chemical/plantower,pms7003.txt| 20 ++
>> > .../devicetree/bindings/vendor-prefixes.txt | 1 +
>> > drivers/iio/adc/ad7766.c | 22 +-
>> > drivers/iio/chemical/Kconfig | 10 +
>> > drivers/iio/chemical/Makefile | 1 +
>> > drivers/iio/chemical/pms7003.c| 340 ++
>> > drivers/iio/chemical/sps30.c | 3 +
>> > 8 files changed, 388 insertions(+), 20 deletions(-)
>> > create mode 100644
>> > Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
>> > create mode 100644 drivers/iio/chemical/pms7003.c
>> >
>> > diff --git
>> > a/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
>> > b/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
>> > index 1b8132cd9060..eb939fe77836 100644
>> > --- a/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
>> > +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
>> > @@ -14,11 +14,6 @@ Optional properties:
>> >vref-supply is not added the ADC will use
>> > internal voltage
>> >reference.
>> >
>> > -Required Node in the NPCM7xx BMC:
>> > -An additional register is present in the NPCM7xx SOC which is
>> > -assumed to be in the same device tree, with and marked as
>> > -compatible with "nuvoton,npcm750-rst".
>> > -
>> > Example:
>> >
>> > adc: adc@f000c000 {
>> > @@ -27,9 +22,3 @@ adc: adc@f000c000 {
>> > interrupts = ;
>> > clocks = <&clk NPCM7XX_CLK_ADC>;
>> > };
>> > -
>> > -rst: rst@f0801000 {
>> > - compatible = "nuvoton,npcm750-rst", "syscon",
>> > - "simple-mfd";
>> > - reg = <0xf0801000 0x6C>;
>> > -};
>> > diff --git
>> > a/Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
>> > b/Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
>> > new file mode 100644
>> > index ..7b5f06f324c8
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings
Re: [PATCH v2] iio: adc: ad7766: Change alignment to match paranthesis
On Tue, Apr 23, 2019 at 11:04 PM Camylla Gonçalves Cantanheide
wrote:
>
> This commit align broken line to match upper line parenthesis,
> in lines 80, 130, 237, 242, 255, 264 and 293. Solves the checkpatch.pl's
> message:
>
> CHECK: Alignment should match open parenthesis
>
>In lines 130, 255, 264 and 293 it was necessary to break a line.
>
This patch is errounous; seems it wasn't generated correctly.
It does too many things in one commit.
> Signed-off-by: Camylla Gonçalves Cantanheide
> ---
> .../bindings/iio/adc/nuvoton,npcm-adc.txt | 11 -
> .../iio/chemical/plantower,pms7003.txt| 20 ++
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> drivers/iio/adc/ad7766.c | 22 +-
> drivers/iio/chemical/Kconfig | 10 +
> drivers/iio/chemical/Makefile | 1 +
> drivers/iio/chemical/pms7003.c| 340 ++
> drivers/iio/chemical/sps30.c | 3 +
> 8 files changed, 388 insertions(+), 20 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
> create mode 100644 drivers/iio/chemical/pms7003.c
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
> b/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
> index 1b8132cd9060..eb939fe77836 100644
> --- a/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
> @@ -14,11 +14,6 @@ Optional properties:
>vref-supply is not added the ADC will use internal
> voltage
>reference.
>
> -Required Node in the NPCM7xx BMC:
> -An additional register is present in the NPCM7xx SOC which is
> -assumed to be in the same device tree, with and marked as
> -compatible with "nuvoton,npcm750-rst".
> -
> Example:
>
> adc: adc@f000c000 {
> @@ -27,9 +22,3 @@ adc: adc@f000c000 {
> interrupts = ;
> clocks = <&clk NPCM7XX_CLK_ADC>;
> };
> -
> -rst: rst@f0801000 {
> - compatible = "nuvoton,npcm750-rst", "syscon",
> - "simple-mfd";
> - reg = <0xf0801000 0x6C>;
> -};
> diff --git
> a/Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
> b/Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
> new file mode 100644
> index ..7b5f06f324c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
> @@ -0,0 +1,20 @@
> +* Plantower PMS7003 particulate matter sensor
> +
> +Required properties:
> +- compatible: must be "plantower,pms7003"
> +- vcc-supply: phandle to the regulator that provides power to the sensor
> +
> +Optional properties:
> +- plantower,set-gpios: phandle to the GPIO connected to the SET line
> +- reset-gpios: phandle to the GPIO connected to the RESET line
> +
> +Refer to serial/slave-device.txt for generic serial attached device bindings.
> +
> +Example:
> +
> +&uart0 {
> + air-pollution-sensor {
> + compatible = "plantower,pms7003";
> + vcc-supply = <®_vcc5v0>;
> + };
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
> b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 389508584f48..42816baeb381 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -304,6 +304,7 @@ phytec PHYTEC Messtechnik GmbH
> picochip Picochip Ltd
> pine64 Pine64
> pixcir PIXCIR MICROELECTRONICS Co., Ltd
> +plantower Plantower Co., Ltd
> plathome Plat'Home Co., Ltd.
> plda PLDA
> plxBroadcom Corporation (formerly PLX Technology)
> diff --git a/drivers/iio/adc/ad7766.c b/drivers/iio/adc/ad7766.c
> index 3ae14fc8c649..101502435768 100644
> --- a/drivers/iio/adc/ad7766.c
> +++ b/drivers/iio/adc/ad7766.c
> @@ -77,7 +77,7 @@ static irqreturn_t ad7766_trigger_handler(int irq, void *p)
> goto done;
>
> iio_push_to_buffers_with_timestamp(indio_dev, ad7766->data,
> - pf->timestamp);
> + pf->timestamp);
> done:
> iio_trigger_notify_done(indio_dev->trig);
>
> @@ -127,7 +127,8 @@ static int ad7766_postdisable(struct iio_dev *indio_dev)
> }
>
> static int ad7766_read_raw(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan, int *val, int *val2, long info)
> + const struct iio_chan_spec *chan, int *val,
> + int *val2, long info)
> {
> struct ad7766 *ad7766 = iio_priv(indio_dev);
> struct regulator *vref = ad7766->reg[AD7766_SUPPLY_VREF].consumer;
> @@ -234,12 +235,12 @@ static int ad7766_probe(struct spi_device *spi)
> ad7766->reg[AD7766_SUPPLY_VREF].supply = "vref";
>
> ret = devm_regulator_bulk_get(&spi->dev, ARRAY_SIZE(ad7766->reg),
> - ad7766->reg);
> +
Re: [PATCH] Wqiio: adc: ad7766: Change alignment to match paranthesis
On Tue, Apr 23, 2019 at 11:01 PM Camylla Gonçalves Cantanheide
wrote:
>
> This commit align broken line to match upper line parenthesis,
> in lines 80, 130, 237, 242, 255, 264 and 293. Solves the checkpatch.pl's
> message:
>
> CHECK: Alignment should match open parenthesis
>
> In lines 130, 255, 264 and 293 it was necessary to break a line.
>
This patch seems duplicate.
I can't tell if it's different from the other.
Has the same problem.
> Signed-off-by: Camylla Gonçalves Cantanheide
> ---
> .../bindings/iio/adc/nuvoton,npcm-adc.txt | 11 -
> .../iio/chemical/plantower,pms7003.txt| 20 ++
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> drivers/iio/adc/ad7766.c | 22 +-
> drivers/iio/chemical/Kconfig | 10 +
> drivers/iio/chemical/Makefile | 1 +
> drivers/iio/chemical/pms7003.c| 340 ++
> drivers/iio/chemical/sps30.c | 3 +
> 8 files changed, 388 insertions(+), 20 deletions(-)
> create mode 100644
> Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
> create mode 100644 drivers/iio/chemical/pms7003.c
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
> b/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
> index 1b8132cd9060..eb939fe77836 100644
> --- a/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,npcm-adc.txt
> @@ -14,11 +14,6 @@ Optional properties:
>vref-supply is not added the ADC will use internal
> voltage
>reference.
>
> -Required Node in the NPCM7xx BMC:
> -An additional register is present in the NPCM7xx SOC which is
> -assumed to be in the same device tree, with and marked as
> -compatible with "nuvoton,npcm750-rst".
> -
> Example:
>
> adc: adc@f000c000 {
> @@ -27,9 +22,3 @@ adc: adc@f000c000 {
> interrupts = ;
> clocks = <&clk NPCM7XX_CLK_ADC>;
> };
> -
> -rst: rst@f0801000 {
> - compatible = "nuvoton,npcm750-rst", "syscon",
> - "simple-mfd";
> - reg = <0xf0801000 0x6C>;
> -};
> diff --git
> a/Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
> b/Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
> new file mode 100644
> index ..7b5f06f324c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/chemical/plantower,pms7003.txt
> @@ -0,0 +1,20 @@
> +* Plantower PMS7003 particulate matter sensor
> +
> +Required properties:
> +- compatible: must be "plantower,pms7003"
> +- vcc-supply: phandle to the regulator that provides power to the sensor
> +
> +Optional properties:
> +- plantower,set-gpios: phandle to the GPIO connected to the SET line
> +- reset-gpios: phandle to the GPIO connected to the RESET line
> +
> +Refer to serial/slave-device.txt for generic serial attached device bindings.
> +
> +Example:
> +
> +&uart0 {
> + air-pollution-sensor {
> + compatible = "plantower,pms7003";
> + vcc-supply = <®_vcc5v0>;
> + };
> +};
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
> b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 389508584f48..42816baeb381 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -304,6 +304,7 @@ phytec PHYTEC Messtechnik GmbH
> picochip Picochip Ltd
> pine64 Pine64
> pixcir PIXCIR MICROELECTRONICS Co., Ltd
> +plantower Plantower Co., Ltd
> plathome Plat'Home Co., Ltd.
> plda PLDA
> plxBroadcom Corporation (formerly PLX Technology)
> diff --git a/drivers/iio/adc/ad7766.c b/drivers/iio/adc/ad7766.c
> index 3ae14fc8c649..101502435768 100644
> --- a/drivers/iio/adc/ad7766.c
> +++ b/drivers/iio/adc/ad7766.c
> @@ -77,7 +77,7 @@ static irqreturn_t ad7766_trigger_handler(int irq, void *p)
> goto done;
>
> iio_push_to_buffers_with_timestamp(indio_dev, ad7766->data,
> - pf->timestamp);
> + pf->timestamp);
> done:
> iio_trigger_notify_done(indio_dev->trig);
>
> @@ -127,7 +127,8 @@ static int ad7766_postdisable(struct iio_dev *indio_dev)
> }
>
> static int ad7766_read_raw(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan, int *val, int *val2, long info)
> + const struct iio_chan_spec *chan, int *val,
> + int *val2, long info)
> {
> struct ad7766 *ad7766 = iio_priv(indio_dev);
> struct regulator *vref = ad7766->reg[AD7766_SUPPLY_VREF].consumer;
> @@ -234,12 +235,12 @@ static int ad7766_probe(struct spi_device *spi)
> ad7766->reg[AD7766_SUPPLY_VREF].supply = "vref";
>
> ret = devm_regulator_bulk_get(&spi->dev, ARRAY_SIZE(ad7766->reg),
> - ad7766->reg);
> +
[PATCH 2/2][V2] iio: Handle enumerated properties with gaps
From: Lars-Peter Clausen
Some enums might have gaps or reserved values in the middle of their value
range. E.g. consider a 2-bit enum where the values 0, 1 and 3 have a
meaning, but 2 is a reserved value and can not be used.
Add support for such enums to the IIO enum helper functions. A reserved
values is marked by setting its entry in the items array to NULL rather
than the normal descriptive string value.
Signed-off-by: Lars-Peter Clausen
Signed-off-by: Alexandru Ardelean
---
drivers/iio/industrialio-core.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index f2ebca65f964..e848b35bd9c0 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -447,8 +447,11 @@ ssize_t iio_enum_available_read(struct iio_dev *indio_dev,
if (!e->num_items)
return 0;
- for (i = 0; i < e->num_items; ++i)
+ for (i = 0; i < e->num_items; ++i) {
+ if (!e->items[i])
+ continue;
len += scnprintf(buf + len, PAGE_SIZE - len, "%s ",
e->items[i]);
+ }
/* replace last space with a newline */
buf[len - 1] = '\n';
@@ -469,7 +472,7 @@ ssize_t iio_enum_read(struct iio_dev *indio_dev,
i = e->get(indio_dev, chan);
if (i < 0)
return i;
- else if (i >= e->num_items)
+ else if (i >= e->num_items || !e->items[i])
return -EINVAL;
return snprintf(buf, PAGE_SIZE, "%s\n", e->items[i]);
@@ -486,7 +489,7 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev,
if (!e->set)
return -EINVAL;
- ret = __sysfs_match_string(e->items, e->num_items, buf);
+ ret = __sysfs_match_string_with_gaps(e->items, e->num_items, buf);
if (ret < 0)
return ret;
--
2.17.1
[PATCH 1/2][V2] lib: add __sysfs_match_string_with_gaps() helper
This helper is similar to __sysfs_match_string() with the exception that it
ignores NULL elements within the array.
It takes an extra parameter (called `gaps`) which when true will ignore
the NULL elements. When false, this function behaves exactly like
`__sysfs_match_string()`.
Signed-off-by: Alexandru Ardelean
---
include/linux/string.h | 2 ++
lib/string.c | 48 --
2 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index 7927b875f80c..08e4a33b5f29 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -190,6 +190,8 @@ static inline int strtobool(const char *s, bool *res)
int match_string(const char * const *array, size_t n, const char *string);
int __sysfs_match_string(const char * const *array, size_t n, const char *s);
+int __sysfs_match_string_with_gaps(const char * const *array, size_t n,
+ const char *str);
/**
* sysfs_match_string - matches given string in an array
diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..2ae3b7e5ff3d 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -658,32 +658,58 @@ int match_string(const char * const *array, size_t n,
const char *string)
}
EXPORT_SYMBOL(match_string);
-/**
- * __sysfs_match_string - matches given string in an array
- * @array: array of strings
- * @n: number of strings in the array or -1 for NULL terminated arrays
- * @str: string to match with
- *
- * Returns index of @str in the @array or -EINVAL, just like match_string().
- * Uses sysfs_streq instead of strcmp for matching.
- */
-int __sysfs_match_string(const char * const *array, size_t n, const char *str)
+static int __sysfs_match_string_common(const char * const *array, size_t n,
+ const char *str, bool gaps)
{
const char *item;
int index;
for (index = 0; index < n; index++) {
item = array[index];
- if (!item)
+ if (!item) {
+ if (gaps)
+ continue;
break;
+ }
if (sysfs_streq(item, str))
return index;
}
return -EINVAL;
}
+
+/**
+ * __sysfs_match_string - matches given string in an array
+ * @array: array of strings
+ * @n: number of strings in the array or -1 for NULL terminated arrays
+ * @str: string to match with
+ *
+ * Returns index of @str in the @array or -EINVAL, just like match_string().
+ * Uses sysfs_streq instead of strcmp for matching.
+ */
+int __sysfs_match_string(const char * const *array, size_t n, const char *str)
+{
+ return __sysfs_match_string_common(array, n, str, false);
+}
EXPORT_SYMBOL(__sysfs_match_string);
+/**
+ * __sysfs_match_string_with_gaps - matches string in array ignoring NULLs
+ * @array: array of strings
+ * @n: number of strings in the array or -1 for NULL terminated arrays
+ * @str: string to match with
+ *
+ * Returns index of @str in the @array or -EINVAL, just like match_string().
+ * Uses sysfs_streq instead of strcmp for matching.
+ * Ignores NULL entries within the @array.
+ */
+int __sysfs_match_string_with_gaps(const char * const *array, size_t n,
+ const char *str)
+{
+ return __sysfs_match_string_common(array, n, str, true);
+}
+EXPORT_SYMBOL(__sysfs_match_string_with_gaps);
+
#ifndef __HAVE_ARCH_MEMSET
/**
* memset - Fill a region of memory with the given value
--
2.17.1
[PATCH 1/2] lib: add __sysfs_match_string_with_gaps() helper
This helper is similar to __sysfs_match_string() with the exception that it
ignores NULL elements within the array.
It takes an extra parameter (called `gaps`) which when true will ignore
the NULL elements. When false, this function behaves exactly like
`__sysfs_match_string()`.
Signed-off-by: Alexandru Ardelean
---
include/linux/string.h | 9 -
lib/string.c | 14 ++
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/include/linux/string.h b/include/linux/string.h
index 7927b875f80c..30595ed483dc 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -189,7 +189,14 @@ static inline int strtobool(const char *s, bool *res)
}
int match_string(const char * const *array, size_t n, const char *string);
-int __sysfs_match_string(const char * const *array, size_t n, const char *s);
+int __sysfs_match_string_with_gaps(const char * const *array, size_t n,
+ const char *str, bool gaps);
+
+static inline int __sysfs_match_string(const char * const *array, size_t n,
+ const char *str)
+{
+ return __sysfs_match_string_with_gaps(array, n, str, false);
+}
/**
* sysfs_match_string - matches given string in an array
diff --git a/lib/string.c b/lib/string.c
index 38e4ca08e757..8ddac3cd292a 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -659,30 +659,36 @@ int match_string(const char * const *array, size_t n,
const char *string)
EXPORT_SYMBOL(match_string);
/**
- * __sysfs_match_string - matches given string in an array
+ * __sysfs_match_string_with_gaps - matches string in array ignoring NULLs
* @array: array of strings
* @n: number of strings in the array or -1 for NULL terminated arrays
* @str: string to match with
+ * @gaps: boolean to ignore NULL elements within the array
*
* Returns index of @str in the @array or -EINVAL, just like match_string().
* Uses sysfs_streq instead of strcmp for matching.
+ * Ignores NULL pointers within the @array if @gaps is true.
*/
-int __sysfs_match_string(const char * const *array, size_t n, const char *str)
+int __sysfs_match_string_with_gaps(const char * const *array, size_t n,
+ const char *str, bool gaps)
{
const char *item;
int index;
for (index = 0; index < n; index++) {
item = array[index];
- if (!item)
+ if (!item) {
+ if (gaps)
+ continue;
break;
+ }
if (sysfs_streq(item, str))
return index;
}
return -EINVAL;
}
-EXPORT_SYMBOL(__sysfs_match_string);
+EXPORT_SYMBOL(__sysfs_match_string_with_gaps);
#ifndef __HAVE_ARCH_MEMSET
/**
--
2.17.1
[PATCH 2/2] iio: Handle enumerated properties with gaps
From: Lars-Peter Clausen
Some enums might have gaps or reserved values in the middle of their value
range. E.g. consider a 2-bit enum where the values 0, 1 and 3 have a
meaning, but 2 is a reserved value and can not be used.
Add support for such enums to the IIO enum helper functions. A reserved
values is marked by setting its entry in the items array to NULL rather
than the normal descriptive string value.
Signed-off-by: Lars-Peter Clausen
Signed-off-by: Alexandru Ardelean
---
drivers/iio/industrialio-core.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index f2ebca65f964..c141a29bf446 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -447,8 +447,11 @@ ssize_t iio_enum_available_read(struct iio_dev *indio_dev,
if (!e->num_items)
return 0;
- for (i = 0; i < e->num_items; ++i)
+ for (i = 0; i < e->num_items; ++i) {
+ if (!e->items[i])
+ continue;
len += scnprintf(buf + len, PAGE_SIZE - len, "%s ",
e->items[i]);
+ }
/* replace last space with a newline */
buf[len - 1] = '\n';
@@ -469,7 +472,7 @@ ssize_t iio_enum_read(struct iio_dev *indio_dev,
i = e->get(indio_dev, chan);
if (i < 0)
return i;
- else if (i >= e->num_items)
+ else if (i >= e->num_items || !e->items[i])
return -EINVAL;
return snprintf(buf, PAGE_SIZE, "%s\n", e->items[i]);
@@ -486,7 +489,8 @@ ssize_t iio_enum_write(struct iio_dev *indio_dev,
if (!e->set)
return -EINVAL;
- ret = __sysfs_match_string(e->items, e->num_items, buf);
+ ret = __sysfs_match_string_with_gaps(e->items, e->num_items,
+buf, true);
if (ret < 0)
return ret;
--
2.17.1
Re: linux-next: manual merge of the staging tree with the staging.current tree
On Tue, Apr 9, 2019 at 6:40 PM Andy Shevchenko wrote: > > On Mon, Apr 08, 2019 at 01:01:51PM +0100, Jonathan Cameron wrote: > > On Mon, 8 Apr 2019 13:34:37 +0300 > > Andy Shevchenko wrote: > > > On Mon, Apr 08, 2019 at 11:14:39AM +0100, Jonathan Cameron wrote: > > > > On Mon, 8 Apr 2019 13:01:21 +0300 > > > > Andy Shevchenko wrote: > > > > > On Mon, Apr 08, 2019 at 09:14:58AM +0100, Jonathan Cameron wrote: > > > > > > On Mon, 8 Apr 2019 13:02:12 +1000 > > > > > > Stephen Rothwell wrote: > > > > > > > That is the correct resolution. > > > > > > > > > > I think it still misses the following fix: > > > > > > > Is that actually a problem given it's copied over from > > > > buffer->scan_mask just after allocation? > > > > The two masks are the same length so I don't think we have a problem > > > > with this one. > > > > Am I missing something? > > > > > > Hmm... I didn't get why the commit 20ea39ef9f2f fixes anything. > > > > > Good point. I'm don't think it ever did. > > > > Alex, any thoughts? > Hey, This seems to have been in the context of our tree. We have this patch: https://github.com/analogdevicesinc/linux/commit/81d00795b1537 That removes bitmap_copy() . See here: https://github.com/analogdevicesinc/linux/commit/81d00795b1537#diff-0a87744ce945d2c1c89ea19f21fb35bbL397 This change is not upstreamed yet. I guess I am slowly going nuts with trying to sync multiple trees [ our master, upstream IIO & some internal temp-branches ]. To give a bit of background: we've noticed this weird behavior while testing a AD7193 chip with the AD7192 driver and some weird things were happening. At the time, this patch seemed easy to send upstream, so I sent it. Sorry for the noise. I guess the conclusion is, that in the context of the mainline IIO tree, commit 20ea39ef9f2f is not needed. Thanks Alex > I have a thought that it might be possible that somewhere code is still > broken, > i.e. accessing bitmap behind the size (for example, iterating by unsigned long > without bitmap size being aligned to size of unsigned long). > > If this is a case, the mentioned patch has a symptomatic healing and not > fixing > a root cause. > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH] staging: iio: cdc: ad7746: Replace bitshift by BIT
On Wed, Apr 3, 2019 at 11:46 PM Lucas Oshiro wrote: > > Replace bitshifts on lines 54, 56 and 78 of ad7746.c. > Hey, This is only partially done. If doing conversions to BIT(x) macro, I would say to do them for all cases. Thanks Alex > Signed-off-by: Lucas Oshiro > --- > drivers/staging/iio/cdc/ad7746.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7746.c > b/drivers/staging/iio/cdc/ad7746.c > index 0eb28fea876e..ea48b14cee72 100644 > --- a/drivers/staging/iio/cdc/ad7746.c > +++ b/drivers/staging/iio/cdc/ad7746.c > @@ -51,9 +51,9 @@ > #define AD7746_CAPSETUP_CACHOP BIT(0) > > /* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) > */ > -#define AD7746_VTSETUP_VTEN(1 << 7) > +#define AD7746_VTSETUP_VTENBIT(7) > #define AD7746_VTSETUP_VTMD_INT_TEMP (0 << 5) > -#define AD7746_VTSETUP_VTMD_EXT_TEMP (1 << 5) > +#define AD7746_VTSETUP_VTMD_EXT_TEMP BIT(5) > #define AD7746_VTSETUP_VTMD_VDD_MON(2 << 5) > #define AD7746_VTSETUP_VTMD_EXT_VIN(3 << 5) > #define AD7746_VTSETUP_EXTREF BIT(4) > @@ -75,7 +75,7 @@ > #define AD7746_CONF_VTFS_MASK GENMASK(7, 6) > #define AD7746_CONF_CAPFS_MASK GENMASK(5, 3) > #define AD7746_CONF_MODE_IDLE (0 << 0) > -#define AD7746_CONF_MODE_CONT_CONV (1 << 0) > +#define AD7746_CONF_MODE_CONT_CONV BIT(0) > #define AD7746_CONF_MODE_SINGLE_CONV (2 << 0) > #define AD7746_CONF_MODE_PWRDN (3 << 0) > #define AD7746_CONF_MODE_OFFS_CAL (5 << 0) > -- > 2.21.0 >
Re: Work on iio: stating: frequency: ad9832
On Mon, Apr 1, 2019 at 7:13 PM Jonathan Cameron wrote: > > On Mon, 1 Apr 2019 11:25:29 -0300 > Marcelo Schmitt wrote: > > > Hello, > > > > I was looking for some work on staging: iio: ad9832 and made some > > observations while reading the driver. > > > > Apparently it had no devicetree documentation so I tried to elaborate > > one. > > It uses a platform_data variable to load external clock > > frequency (I tried to make it use linux's clock framework). > Good. > > > Some device attributes don't seem to be standardized on > > Documentation/ABI/testing/sysfs-bus-iio and there's no specific ABI > > for ad9832 nearby nor at staging/iio/Documentation. So maybe those > > missing ABI could be documented. > Beware. It's an old driver, so it may be that we actually want to change > it's ABI rather than documenting what is there (I have haven't looked!) > This one can actually be coupled a bit with the AD9834 driver. There's been some work on trying to move that one out of staging as well. You can take a look at the patches sent for that driver. They should be find-able on patchwork https://patchwork.kernel.org/project/linux-iio/list/?series=&submitter=&state=*&q=ad9834&archive=both&delegate= There are ideas worth borrowing from there. The issue with the AD9834 [if i recall correctly] is that it doesn't quite fit the classical IIO channel model. Meaning, you can only activate the output of one channel at one moment in time, and not both. > > The device has to set some internal registers to operate correctly, > > AD9832_FREQXHM and AD9832_PHASEXH, would it be feasible to set iio > > chanels for this? > > What are they? If they correspond to output channels in some sensible > way then maybe... > > > I couldn't understand why checkpatch.pl gave errors on IIO_DEV_ATTR_* > > macros. To me they seem to have no problem. > > Also it has that platform_data to be moved to include/linux/iio. Is > > there any special reason for it not being there already? Which are > > the criterions a platform_data need to satisfy to be put there? > A driver moving out of staging shouldn't have platform data. It needs > to be converted over to more modern mechanisms. We don't have a problem > supporting platform data for devices that have old school device files > already in tree, but that shouldn't be the case for a driver in staging. > > Hence we can clean it up and move forward with just DT bindings. > > > > I'm sending a patchset with some things I've already done. > Cool. I'll look at those later in the week if no one beats me to them. > > > > > Is there something else that could be done in this device driver? > > Please, tell if I've forgotten something. > > I'll take a look, but it may be a little while before I do. > Hopefully someone else gets there first! > > Jonathan > > > > > Any advice is welcome. > > Thanks, > > > > Marcelo > >
Re: [PATCH 4/4] staging: iio: ad9832: add devicetree documentation
On Mon, Apr 1, 2019 at 5:38 PM Marcelo Schmitt
wrote:
>
> Add a devicetree documentation for the ad9832 direct digital
> synthesizer, waveform generator.
>
> Signed-off-by: Marcelo Schmitt
> ---
> .../bindings/iio/frequency/ad9832.txt | 26 +++
> 1 file changed, 26 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/frequency/ad9832.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> b/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> new file mode 100644
> index ..6a35fdff5a48
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> @@ -0,0 +1,26 @@
> +Analog Devices AD9832 Direct Digital Synthesizer, Waveform Generator
> +
> +Data sheet:
> +https://www.analog.com/media/en/technical-documentation/data-sheets/AD9832.pdf
> +
> +Required properties:
> + - compatible : Must be "adi,ad9832"
> + - reg : SPI chip select number for the device
> + - spi-max-frequency = Max SPI frequency to use (< 2500)
> + - clocks : The clock reference for the DDS output
> + - clock-names : Must be "mclk"
It's always a good idea to reference other base dt docs.
For SPI you could:
```
For more information on SPI properties, please consult
Documentation/devicetree/bindings/spi/spi-bus.txt
```
For clock:
```
For more information on clock bindings properties, please consult
Documentation/devicetree/bindings/clock/clock-bindings.txt
```
For regulator:
```
For more information on regulator bindings properties, please consult
Documentation/devicetree/bindings/regulator/regulator.txt
```
> +
> +Optional properties:
> + - avdd-supply: Definition of the regulator used as analog supply
> + - dvdd-supply : Definition of the regulator used as digital supply
> +
> +Example:
> + adi9832-dds@0 {
> + compatible = "adi,ad9832";
> + reg = <0>;
> + spi-max-frequency = <2500>;
> + clocks = <&ad9832_mclk>;
> + clock-names = "mclk";
> + avdd-suppy = <&avdd>;
> + dvdd-suppy = <&dvdd>;
> + };
> --
> 2.20.1
>
Re: Help on testing ad5933 driver
On Thu, Mar 21, 2019 at 9:39 PM Marcelo Schmitt wrote: > > Hello, would anyone mind helping me test ad5933 driver on actual > hardware? I went through this > (https://oslongjourney.github.io/linux-kernel/experiment-one-iio-dummy/) > tutorial so I was able to load iio_simple_dummy driver, create and > inspect some dummy devices. Now, as Jonathan has asked me, I would like > to test ad5933 driver on an EVAL-AD5933 board which was donated to FLUSP > (https://flusp.ime.usp.br/). > > So far I've been hesitating to plug this device on my Debian distro > since this > (https://www.analog.com/media/en/technical-documentation/user-guides/UG-364.pdf) > user guide for Windows says not to connect it before driver > installation. Is there something that could harm the board if plugged > on a computer without a proper driver? > You should take into account that a lot of eval boards have their eval SW written for Windows. This is something of a legacy-thing, because most corporations have been running their computers (for work & dev & offices) on Windows. So, you shouldn't take things ad-literam (to the letter) when reading stuff for Windows and when writing code for Linux. > I also didn't understand the hardware configuration showed on this > (https://wiki.analog.com/resources/tools-software/linux-drivers/iio-impedance-analyzer/ad5933) > page. Hmm, that doc was written a while ago. The newer eval board doesn't look like the one in the wiki. Also, since the eval SW was targeted for Windows, getting it to work for Linux (and IIO) implies some hacking/reverse-engineering. The reason for this (reverse-engineering) is because [traditionally] eval boards are meant to highlight characteristics of the chip, so if using Windows, this should be simple. Unfortunately, the docs aren't helping in this case. So, in this case, I would get some volt-meter & oscilloscope to help. It looks to me that U2 is the AD5933 on the eval-board. Worst case, you can solder directly to the pins and link them to a Raspberry PI [on the I2C], power, ground, etc. But, you can take a look at the T1 to T8 (if I didn't miss anything) and connect to them, and see what each of them is for. Hopefully, one of those test-points is for I2C, in which case you can attach wires to them and connect them to a host. I did not find a good doc for them yet. But anyway, I would ask some HW guy to help here (because I'm a SW guy mostly),and get help on figuring out the eval board > > Any advice will be greatly appreciated. > Thanks in advance, > > Marcelo
Re: [PATCH v3 5/7] staging: iio: ad5933: add ABI documentation
On Tue, Mar 12, 2019 at 7:31 PM Marcelo Schmitt wrote: > > On 03/11, Alexandru Ardelean wrote: > > On Sun, Mar 10, 2019 at 7:47 PM Marcelo Schmitt > > wrote: > > > > > > Add an ABI documentation for the ad5933 driver. > > > > There's already an ABI documentation for this driver. > > See: > > https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/drivers/staging/iio/Documentation/sysfs-bus-iio-impedance-analyzer-ad5933 > > > > Apologies for not mentioning this sooner. > > > > Could you check for other references for ad5933 in the kernel (in staging) ? > > I don't think I missed other stuff, but it's good to check. > > > > Didn't see any other reference for ad5933 with find and grep inside iio, > neither on general documentation. I'll replace mine ABI doc by yours and > add your sign-off-by on my next patch set. OK? You don't need to add me anywhere :) I did not contribute anything to that ABI doc. I just raised a note about it. But, that doc should be taken care of; whether you update it, remove it, or anything. It's good to not forget it in staging. > > > > > > > Signed-off-by: Marcelo Schmitt > > > --- > > > .../ABI/testing/sysfs-bus-iio-ad5933 | 39 +++ > > > 1 file changed, 39 insertions(+) > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-ad5933 > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > > > b/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > > > new file mode 100644 > > > index ..8a60dd178b1f > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > > > @@ -0,0 +1,39 @@ > > > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_start > > > +Date: March 2019 > > > +KernelVersion: Kernel 4.19 > > > +Contact: linux-...@vger.kernel.org > > > +Description: > > > + The start frequency. Set this to define the frequency > > > point at > > > + which the device should start the next frequency sweep. > > > + > > > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_increment > > > +Date: March 2019 > > > +KernelVersion: Kernel 4.19 > > > +Contact: linux-...@vger.kernel.org > > > +Description: > > > + The frequency sweep increment. Set this to define the > > > amount by > > > + which the frequency is incremented after each scan point. > > > After > > > + the measurement at a frequency point is completed, the > > > next > > > + measurement will be made at a frequency point > > > + 'frequency increment'Hz higher than the previous one > > > unless a > > > + repeat frequency command is issued. This behavior will > > > follow > > > + until the defined number of frequency points have been > > > measured > > > + or frequency sweep is somewhat reset. > > > + > > > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_points > > > +Date: March 2019 > > > +KernelVersion: Kernel 4.19 > > > +Contact: linux-...@vger.kernel.org > > > +Description: > > > + The number of increments. This defines the number of > > > frequency > > > + points in the frequency sweep. > > > + > > > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_settling_cycles > > > +Date: March 2019 > > > +KernelVersion: Kernel 4.19 > > > +Contact: linux-...@vger.kernel.org > > > +Description: > > > + Number of settling time cycles. This sets the delay > > > between a > > > + start frequency sweep/increment frequency /repeat > > > frequency to > > > + be proportional to the excitation signal frequency times > > > the > > > + number of settling time cycles. > > > -- > > > 2.20.1 > > >
Re: [PATCH v3 5/7] staging: iio: ad5933: add ABI documentation
On Sun, Mar 10, 2019 at 7:47 PM Marcelo Schmitt wrote: > > Add an ABI documentation for the ad5933 driver. There's already an ABI documentation for this driver. See: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/drivers/staging/iio/Documentation/sysfs-bus-iio-impedance-analyzer-ad5933 Apologies for not mentioning this sooner. Could you check for other references for ad5933 in the kernel (in staging) ? I don't think I missed other stuff, but it's good to check. > > Signed-off-by: Marcelo Schmitt > --- > .../ABI/testing/sysfs-bus-iio-ad5933 | 39 +++ > 1 file changed, 39 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-ad5933 > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > b/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > new file mode 100644 > index ..8a60dd178b1f > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > @@ -0,0 +1,39 @@ > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_start > +Date: March 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + The start frequency. Set this to define the frequency point at > + which the device should start the next frequency sweep. > + > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_increment > +Date: March 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + The frequency sweep increment. Set this to define the amount > by > + which the frequency is incremented after each scan point. > After > + the measurement at a frequency point is completed, the next > + measurement will be made at a frequency point > + 'frequency increment'Hz higher than the previous one unless a > + repeat frequency command is issued. This behavior will follow > + until the defined number of frequency points have been > measured > + or frequency sweep is somewhat reset. > + > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_points > +Date: March 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + The number of increments. This defines the number of frequency > + points in the frequency sweep. > + > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_settling_cycles > +Date: March 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + Number of settling time cycles. This sets the delay between a > + start frequency sweep/increment frequency /repeat frequency to > + be proportional to the excitation signal frequency times the > + number of settling time cycles. > -- > 2.20.1 >
Re: [PATCH] staging: iio: adc: ad7192: Add spaces around minus operator
On Sun, Mar 10, 2019 at 11:23 PM Karen Palacio wrote: > > Add spaces around minus operator to fix readibility. > > Signed-off-by: Karen Palacio > --- > drivers/staging/iio/adc/ad7192.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/adc/ad7192.c > b/drivers/staging/iio/adc/ad7192.c > index acdbc07..7c632cf 100644 > --- a/drivers/staging/iio/adc/ad7192.c > +++ b/drivers/staging/iio/adc/ad7192.c > @@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev, > } > > static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available, > -in_voltage-voltage_scale_available, > +in_voltage - voltage_scale_available, This isn't broken, but I do agree it should be addressed. I think it's the second time I see a similar patch trying to fix this. So, obviously the code is a bit misleading. > 0444, ad7192_show_scale_available, NULL, 0); > > static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444, > -- > 2.7.4 >
Re: [PATCH v4 0/9] staging: iio: ad7780: move out of staging
On Sun, Mar 3, 2019 at 3:52 PM Renato Lui Geh wrote: > > Hi Alexandru, > > Thanks for the review. Some questions inline. > > Thanks, > Renato > > On 03/01, Ardelean, Alexandru wrote: > >On Thu, 2019-02-28 at 11:23 -0300, Renato Lui Geh wrote: > >> > > > >The patch-series is a bit big. > >I guess that the intent is to move this out-of-staging, but various patches > >are holding this in it's place. > >For patch series above a certain size, you could get many re-spins > >[V2,3,4... so on]. > > > >You could send some of the changes as individual patches, or group them in > >series of 1,2 or 3 patches. That way, you "parallelize" patch sending, and > >when you get reviews on each patch, you can re-spin them individually. > >You'll find over time that certain patches get accepted on V1, others on V2 > >and some on V7 [ hopefully, there isn't any frustration at that point ]. > > On these subseries, should versioning follow this patchset (v5) or should > they start anew (v1), ignoring this series version? I guess, in this case it's fine to leave it as is [in this series]. The series has been reviewed now. But [for me typically], I delay doing a review if a patch-series is longer than 4-5 patches. And I think some reviewers may do the same. So, if I want more people to review/look at my code, I try to make things as easy to review, as possible. And one way, is to definitely keep things decoupled. If one patch can be independent of another [for the same driver/code], I send them as separate patches. This [of course], is a preference. Some reviewers don't mind longer series [than 4-5 patches]. > > > >Well, this is a technique I use to distribute some of my upstream-patch- > >work, so that I can switch easier between internal-work & upstreaming-work. > > > >Coming back to this patch-series. > >My general input, is that the patches are fine over-all; some are just > >cosmetics/noise/a-different-way-of-doing-things-for-this-driver, and those > >usually can be left to preference [of the maintainer usually]. > > > >I do suggest to not hurry when re-spinning patches, and not change too much > >the number of patches in a new series. That can complicate things > >sometimes. But, if doing small patch-series or individual patches, you > >won't have this problem too much. > > > >Thanks > >Alex > > > >> > >> This series of patches contains the following: > >> - Adds user input for the 'gain' and 'filter' GPIO pins for the ad778x > >>family chips; > >> - Filter reading for the ad778x; > >> - Sets pattern macro values and mask for PATTERN status bits; > >> - Adds ID values for the ad7170, ad7171, ad7780 and ad7781 for ID > >>status bits checking; > >> - Moves regulator initialization to after GPIO init to maintain > >>consistency between probe and remove; > >> - Copyright edits, adding SPDX identifier and new copyright holder; > >> - Moves the ad7780 driver out of staging to the mainline; > >> - Adds device tree binding for the ad7780 driver. > >> > >> Renato Lui Geh (9): > >> staging: iio: ad7780: add gain & filter gpio support > >> staging: iio: ad7780: add filter reading to ad778x > >> staging: iio: ad7780: set pattern values and masks directly > >> staging:iio:ad7780: add chip ID values and mask > >> staging: iio: ad7780: move regulator to after GPIO init > >> staging: iio: ad7780: add SPDX identifier > >> staging: iio: ad7780: add new copyright holder > >> staging: iio: ad7780: moving ad7780 out of staging > >> staging: iio: ad7780: add device tree binding > >> > >> Changelog: > >> *v3 > >> - SPDX and regulator init as patches > >> - Renamed filter to odr and ad778x_filter to ad778x_odr_avail > >> - Removed unnecessary regulator disabling > >> - Removed unnecessary AD_SD_CHANNEL macro > >> - Changed unsigned int to unsigned long long to avoid overflow > >> *v4 > >> - Split gain & filter patch into two, with the new commit adding only > >>filter reading > >> - Changed pattern values to direct values, and added pattern mask > >> - Added ID values and mask > >> - Added new copyright holder > >> - Added device tree binding to the ad7780 driver > >> > >> .../bindings/iio/adc/adi,ad7780.txt | 48 +++ > >> drivers/iio/adc/Kconfig | 12 + > >> drivers/iio/adc/Makefile | 1 + > >> drivers/iio/adc/ad7780.c | 365 ++ > >> drivers/staging/iio/adc/Kconfig | 13 - > >> drivers/staging/iio/adc/Makefile | 1 - > >> drivers/staging/iio/adc/ad7780.c | 277 - > >> 7 files changed, 426 insertions(+), 291 deletions(-) > >> create mode 100644 > >> Documentation/devicetree/bindings/iio/adc/adi,ad7780.txt > >> create mode 100644 drivers/iio/adc/ad7780.c > >> delete mode 100644 drivers/staging/iio/adc/ad7780.c > >> > >> -- > >> 2.21.0 > >> > > > >-- > >You received this message because you are subscribed to the Google Groups > >"Kernel USP" gr
[PATCH] lib/scatterlist: introduce sg_nents_for_dma() helper
From: Andy Shevchenko
Sometimes the user needs to split each entry on the mapped scatter list
due to DMA length constrains. This helper returns a number of entities
assuming that each of them is not bigger than supplied maximum length.
Signed-off-by: Andy Shevchenko
Signed-off-by: Alexandru Ardelean
---
Patch was sent in 2016 initially to the DMA engine sub-system.
Link:
https://patchwork.kernel.org/patch/9389821/
This was part of a larger series:
https://patchwork.kernel.org/project/linux-dmaengine/list/?q=sg_nents_for_dma&archive=both&series=&submitter=&delegate=&state=*
I'm not sure if this is supposed to go into DMAEngine or lib/scatterlist.
It doesn't look like lib/scatterlist is managed by DMAEngine, so (by using
the `get_maintainers.pl` script) I'm sending this patch to this group of
parties.
Thanks
Alex
include/linux/scatterlist.h | 1 +
lib/scatterlist.c | 26 ++
2 files changed, 27 insertions(+)
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index b96f0d0b5b8f..4f40455c40e2 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -253,6 +253,7 @@ static inline void sg_init_marker(struct scatterlist *sgl,
int sg_nents(struct scatterlist *sg);
int sg_nents_for_len(struct scatterlist *sg, u64 len);
+int sg_nents_for_dma(struct scatterlist *sgl, unsigned int sglen, size_t len);
struct scatterlist *sg_next(struct scatterlist *);
struct scatterlist *sg_last(struct scatterlist *s, unsigned int);
void sg_init_table(struct scatterlist *, unsigned int);
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index 9ba349e775ef..5a6fc32f485d 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -86,6 +86,32 @@ int sg_nents_for_len(struct scatterlist *sg, u64 len)
}
EXPORT_SYMBOL(sg_nents_for_len);
+/**
+ * sg_nents_for_dma - return count of DMA-capable entries in scatterlist
+ * @sgl: The scatterlist
+ * @sglen: The current number of entries
+ * @len: The maximum length of DMA-capable block
+ *
+ * Description:
+ * Determines the number of entries in @sgl which would be permitted in
+ * DMA-capable transfer if list had been split accordingly, taking into
+ * account chaining as well.
+ *
+ * Returns:
+ * the number of sgl entries needed
+ *
+ **/
+int sg_nents_for_dma(struct scatterlist *sgl, unsigned int sglen, size_t len)
+{
+ struct scatterlist *sg;
+ int i, nents = 0;
+
+ for_each_sg(sgl, sg, sglen, i)
+ nents += DIV_ROUND_UP(sg_dma_len(sg), len);
+ return nents;
+}
+EXPORT_SYMBOL(sg_nents_for_dma);
+
/**
* sg_last - return the last scatterlist entry in a list
* @sgl: First entry in the scatterlist
--
2.17.1
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron wrote: > > On Fri, 25 Jan 2019 22:14:32 -0200 > Rodrigo Ribeiro wrote: > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > > escreveu: > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > escreveu: > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > > wrote: > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > Hey, > > > > > > Hi, > > > > > > Thanks for answering. > > > > > > > A bit curios about this one. > > > > Are you using this chip/driver ? > > > > > > No, I am just a student at USP (University of São Paulo) starting in Linux > > > Kernel and a new member of the USP Linux Kernel group that has contributed > > > for a few months. > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > > marked for removal (since it's still in staging). > > > > But if there are users for this driver, it could be left around for a > > > > while. > > > > > > This is my first patch in Linux Kernel, but if the driver will be > > > removed, I > > > can send a patch for another driver. Is there any driver that I can > > > fix a style warning? > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I truly > > contribute to? > > How about the ad7150? That one is still listed as production. > What do you think Alex, you probably have better visibility on the > status of these parts and their drivers than I do! > > (note I haven't even opened that one for a few years so no idea > what state the driver is in!) > ad7150 is a good alternative. At a first glance over the driver it looks like it could do with some polish/conversions to newer IIO constructs (like IIO triggers, maybe some newer event handling mechanisms?). I'll sync with Stefan [Popa] to see about this stuff at a later point in time. I'd also add here the adxl345 driver. This is mostly informational for anyone who'd find this interesting. There are 2 drivers for this chip, one in IIO [drivers/iio/accel/adxl345] and another one in "drivers/misc/adxl34x.c" as part of the input sub-system. What would be interesting here is to finalize the IIO driver [ I think some features are lacking behind the input driver], and make the input driver a consumer of the IIO driver. >From my side, both these variants are fine to take on. The ad7150 is a good idea as a starter project, and the adxl one is more of a long-term medium-level project. Thanks Alex > Jonathan > > > > > > Thanks > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > escreveu: > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > > wrote: > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > Hey, > > > > > > > > A bit curios about this one. > > > > Are you using this chip/driver ? > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > > marked for removal (since it's still in staging). > > > > But if there are users for this driver, it could be left around for a > > > > while. > > > > > > > > Thanks > > > > Alex > > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > > > Signed-off-by: Rafael Tsuha > > > > > --- > > > > > This macro is not used anywhere. Should we just correct the > > > > > spelling or remove the macro? > > > > > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > > > b/drivers/staging/iio/cdc/ad7152.c > > > > > index 25f51db..c9da6d4 100644 > > > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > > > @@ -35,7 +35,7 @@ > > > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > > > #define AD7152_REG_CH2_SETUP 14 > > > > > #define AD7152_REG_CFG 15 > > > > > -#define AD7152_REG_RESEVERD16 > > > > > +#define AD7152_REG_RESERVED16 > > > > > #define AD7152_REG_CAPDAC_POS 17 > > > > > #define AD7152_REG_CAPDAC_NEG 18 > > > > > #define AD7152_REG_CFG226 > > > > > -- > > > > > 2.7.4 > > > > > >
Re: [PATCH v3] iio: adc: ad7476: Add support for TI ADS786X ADCs
On Mon, Jan 28, 2019 at 11:49 AM Ricardo Ribalda Delgado
wrote:
>
> Add support for Texas Instruments ADS7866, ADS7867 and ADS7868
> 8/10/12 bit Single channel ADC.
>
> Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
>
> Cc: Alexandru Ardelean
> Signed-off-by: Ricardo Ribalda Delgado
> ---
> v3: Changes by Alexandru Ardelean
>
> - Modify Kconfig to clarify that the chip is not from AD, but from TI
>
> drivers/iio/adc/Kconfig | 6 --
> drivers/iio/adc/ad7476.c | 20
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f9354e5ee65c..2d3442252e32 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -57,14 +57,16 @@ config AD7298
> module will be called ad7298.
>
> config AD7476
> - tristate "Analog Devices AD7476 and similar 1-channel ADCs driver"
> + tristate "Analog Devices AD7476 1-channel ADCs driver and other
> similar devices from AD an TI"
> depends on SPI
> select IIO_BUFFER
> select IIO_TRIGGERED_BUFFER
> help
> Say yes here to build support for Analog Devices AD7273, AD7274,
> AD7276,
> AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467,
> AD7468,
> - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters
> (ADC).
> + AD7495, AD7910, AD7920, AD7920. SPI analog to digital converters
> (ADC).
> + And for Texas Instrumments ADS7866, ADS7867, ADS7868. SPI analog to
Typo: Instrumments -> Instruments
> + digital converters (ADC).
I would break it into a table-like format.
I didn't see if there is a common/recommended way of doing things for
these drivers.
But something like:
Say yes here to build support for the following SPI analog
to digital converters (ADCs):
Analog Devices: AD7273, AD7274, AD7276, AD7277, AD7278,
AD7475, AD7476,
AD7477, AD7478, AD7466, AD7467,
AD7468, AD7495, AD7910,
AD7920
Texas Instruments: ADS7866, ADS7867, ADS7868
Funny thing: I just noticed that the AD7920 was listed twice.
>
> To compile this driver as a module, choose M here: the
> module will be called ad7476.
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index 0549686b9ef8..76747488044b 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> ID_ADC081S,
> ID_ADC101S,
> ID_ADC121S,
> + ID_ADS7866,
> + ID_ADS7867,
> + ID_ADS7868,
> };
>
> static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> BIT(IIO_CHAN_INFO_RAW))
> #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> + BIT(IIO_CHAN_INFO_RAW))
>
> static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> [ID_AD7091R] = {
> @@ -209,6 +214,18 @@ static const struct ad7476_chip_info
> ad7476_chip_info_tbl[] = {
> .channel[0] = ADC081S_CHAN(12),
> .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> },
> + [ID_ADS7866] = {
> + .channel[0] = ADS786X_CHAN(12),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> + [ID_ADS7867] = {
> + .channel[0] = ADS786X_CHAN(10),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> + [ID_ADS7868] = {
> + .channel[0] = ADS786X_CHAN(8),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> };
>
> static const struct iio_info ad7476_info = {
> @@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
> {"adc081s", ID_ADC081S},
> {"adc101s", ID_ADC101S},
> {"adc121s", ID_ADC121S},
> + {"ads7866", ID_ADS7866},
> + {"ads7867", ID_ADS7867},
> + {"ads7868", ID_ADS7868},
> {}
> };
> MODULE_DEVICE_TABLE(spi, ad7476_id);
> --
> 2.20.1
>
Re: [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
On Mon, Jan 28, 2019 at 9:43 AM Ricardo Ribalda Delgado
wrote:
>
> HI Alexandru
>
> On Mon, Jan 28, 2019 at 8:38 AM Alexandru Ardelean
> wrote:
> >
> > On Sat, Jan 26, 2019 at 8:21 PM Jonathan Cameron wrote:
> > >
> > > On Fri, 25 Jan 2019 11:04:51 +0100
> > > Ricardo Ribalda Delgado wrote:
> > >
> > > > Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> > > > ADC.
> > > >
> >
> > I don't want this reply be offensive or anything.
> > But since I've seen this precedence, I do have to ask if these driver
> > changes were tested on at least one of the TI chips/eval-boards.
> >
> > Ideally, before adding new drivers it would be nice to make sure that
> > they were run on actual HW.
> > Good code-review can get us only so far.
> >
> > We could ask someone who has any of these chips to test.
> > Or maybe some eval boards from TI could be obtained to test these changes.
> >
> > Overall changes seem good [I haven't taken a close look], but HW
> > testing is king.
> >
>
> I have access to ADS7868 (the 8 bit version) which has been connected
> in "loopback" to the ti-dac7612 (also new driver). I can run any test
> that you need or even give you or someone access (privately) to the
> board via vpn
> or something like that.
>
> Would that work for you?
No, that's fine :)
Thank you for the answer.
And apologies [again] if this was a bit too forward/harsh from my side.
Thanks
Alex
>
> Best regards!
>
> > Thanks
> > Alex
> >
> > > > Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
> > > >
> > > > Signed-off-by: Ricardo Ribalda Delgado
> > > > ---
> > > > v2: I have missnamed the devices
> > > > drivers/iio/adc/Kconfig | 3 ++-
> > > > drivers/iio/adc/ad7476.c | 20
> > > > 2 files changed, 22 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > > > index f9354e5ee65c..d86900fc2634 100644
> > > > --- a/drivers/iio/adc/Kconfig
> > > > +++ b/drivers/iio/adc/Kconfig
> > > > @@ -64,7 +64,8 @@ config AD7476
> > > > help
> > > > Say yes here to build support for Analog Devices AD7273,
> > > > AD7274, AD7276,
> > > > AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467,
> > > > AD7468,
> > > > - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters
> > > > (ADC).
> > > > + AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI
> > > > analog
> > > > + to digital converters (ADC).
> > >
> > > As commented in the earlier thread (after you sent this!), please make
> > > sure
> > > the help text makes it clear that these aren't all Analog devices parts.
> > >
> > > Likely to cause confusion otherwise.
> > >
> > > Good to see you didn't spin another driver but instead tracked down that
> > > this one would work well!
> > >
> > > Thanks,
> > >
> > > Jonathan
> > >
> > > >
> > > > To compile this driver as a module, choose M here: the
> > > > module will be called ad7476.
> > > > diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> > > > index 0549686b9ef8..76747488044b 100644
> > > > --- a/drivers/iio/adc/ad7476.c
> > > > +++ b/drivers/iio/adc/ad7476.c
> > > > @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> > > > ID_ADC081S,
> > > > ID_ADC101S,
> > > > ID_ADC121S,
> > > > + ID_ADS7866,
> > > > + ID_ADS7867,
> > > > + ID_ADS7868,
> > > > };
> > > >
> > > > static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> > > > @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev
> > > > *indio_dev,
> > > > #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> > > > BIT(IIO_CHAN_INFO_RAW))
> > > > #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> > > > +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> > > > + BIT(IIO_CHAN_INFO_RAW))
> > > >
> > > > static const struct ad7476_chip_info ad7476_chip_i
Re: [PATCH v2] iio: adc: ad7476: Add support for ADS786X ADCs
On Sat, Jan 26, 2019 at 8:21 PM Jonathan Cameron wrote:
>
> On Fri, 25 Jan 2019 11:04:51 +0100
> Ricardo Ribalda Delgado wrote:
>
> > Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> > ADC.
> >
I don't want this reply be offensive or anything.
But since I've seen this precedence, I do have to ask if these driver
changes were tested on at least one of the TI chips/eval-boards.
Ideally, before adding new drivers it would be nice to make sure that
they were run on actual HW.
Good code-review can get us only so far.
We could ask someone who has any of these chips to test.
Or maybe some eval boards from TI could be obtained to test these changes.
Overall changes seem good [I haven't taken a close look], but HW
testing is king.
Thanks
Alex
> > Datasheet: http://www.ti.com/lit/ds/symlink/ads7868.pdf
> >
> > Signed-off-by: Ricardo Ribalda Delgado
> > ---
> > v2: I have missnamed the devices
> > drivers/iio/adc/Kconfig | 3 ++-
> > drivers/iio/adc/ad7476.c | 20
> > 2 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index f9354e5ee65c..d86900fc2634 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -64,7 +64,8 @@ config AD7476
> > help
> > Say yes here to build support for Analog Devices AD7273, AD7274,
> > AD7276,
> > AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467,
> > AD7468,
> > - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters
> > (ADC).
> > + AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI analog
> > + to digital converters (ADC).
>
> As commented in the earlier thread (after you sent this!), please make sure
> the help text makes it clear that these aren't all Analog devices parts.
>
> Likely to cause confusion otherwise.
>
> Good to see you didn't spin another driver but instead tracked down that
> this one would work well!
>
> Thanks,
>
> Jonathan
>
> >
> > To compile this driver as a module, choose M here: the
> > module will be called ad7476.
> > diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> > index 0549686b9ef8..76747488044b 100644
> > --- a/drivers/iio/adc/ad7476.c
> > +++ b/drivers/iio/adc/ad7476.c
> > @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> > ID_ADC081S,
> > ID_ADC101S,
> > ID_ADC121S,
> > + ID_ADS7866,
> > + ID_ADS7867,
> > + ID_ADS7868,
> > };
> >
> > static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> > @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> > #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> > BIT(IIO_CHAN_INFO_RAW))
> > #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> > +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits), 12 - (bits), \
> > + BIT(IIO_CHAN_INFO_RAW))
> >
> > static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> > [ID_AD7091R] = {
> > @@ -209,6 +214,18 @@ static const struct ad7476_chip_info
> > ad7476_chip_info_tbl[] = {
> > .channel[0] = ADC081S_CHAN(12),
> > .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > },
> > + [ID_ADS7866] = {
> > + .channel[0] = ADS786X_CHAN(12),
> > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > + },
> > + [ID_ADS7867] = {
> > + .channel[0] = ADS786X_CHAN(10),
> > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > + },
> > + [ID_ADS7868] = {
> > + .channel[0] = ADS786X_CHAN(8),
> > + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> > + },
> > };
> >
> > static const struct iio_info ad7476_info = {
> > @@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
> > {"adc081s", ID_ADC081S},
> > {"adc101s", ID_ADC101S},
> > {"adc121s", ID_ADC121S},
> > + {"ads7866", ID_ADS7866},
> > + {"ads7867", ID_ADS7867},
> > + {"ads7868", ID_ADS7868},
> > {}
> > };
> > MODULE_DEVICE_TABLE(spi, ad7476_id);
>
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Sat, Jan 26, 2019 at 8:09 PM Jonathan Cameron wrote: > > On Fri, 25 Jan 2019 10:19:54 +0200 > Alexandru Ardelean wrote: > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > wrote: > > > > > > Remove the checkpatch.pl check: > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > Hey, > > > > A bit curios about this one. > > Are you using this chip/driver ? > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > marked for removal (since it's still in staging). > > But if there are users for this driver, it could be left around for a while. > > While it might be going away soon, I'm also not that bothered about > the small amount of churn that a tidy up patch like this causes, > and it might not go away ;) > > However it is rather odd to have a 'reserved' entry for a register. > can't see that providing any useful information. Normally I'm > happy to have complete register sets as a form of documentation > if the author wants to do it that way. This however seems silly. > > Alex, we haven't really gone with marking things as 'going away' > before. I'd suggest we take a guess and remove it if you and the > team an analog don't think it's in use. Happy to get a patch for > that if you want to send one. Of course, Rodrigo could do that > patch to get started if that works for everyone? :) > We'll also start a discussion about this particular driver internally. And maybe a procedure for removing drivers that become obsolete [or come close to it]. We also don't/didn't have one for removing "going away" drivers; I just remembered that we took a look over this one and decided not to invest time into it as it's close to being obsolete. Thanks Alex > Jonathan > > > > Thanks > > Alex > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > Signed-off-by: Rafael Tsuha > > > --- > > > This macro is not used anywhere. Should we just correct the > > > spelling or remove the macro? > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > b/drivers/staging/iio/cdc/ad7152.c > > > index 25f51db..c9da6d4 100644 > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > @@ -35,7 +35,7 @@ > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > #define AD7152_REG_CH2_SETUP 14 > > > #define AD7152_REG_CFG 15 > > > -#define AD7152_REG_RESEVERD16 > > > +#define AD7152_REG_RESERVED16 > > > #define AD7152_REG_CAPDAC_POS 17 > > > #define AD7152_REG_CAPDAC_NEG 18 > > > #define AD7152_REG_CFG226 > > > -- > > > 2.7.4 > > > >
Re: [PATCH] iio: adc: ad7476: Add support for ADS786X ADCs
On Thu, Jan 24, 2019 at 11:33 PM Ricardo Ribalda Delgado
wrote:
>
> Add support for ADS7866, ADS7867 and ADS7868 8/10/12 bit Single channel
> ADC.
>
> Datasheet: http://www.ti.com/product/ADS7866
> Datasheet: http://www.ti.com/product/ADS7867
> Datasheet: http://www.ti.com/product/ADS7868
>
> Signed-off-by: Ricardo Ribalda Delgado
> ---
> drivers/iio/adc/Kconfig | 3 ++-
> drivers/iio/adc/ad7476.c | 20
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index f9354e5ee65c..d86900fc2634 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -64,7 +64,8 @@ config AD7476
> help
> Say yes here to build support for Analog Devices AD7273, AD7274,
> AD7276,
> AD7277, AD7278, AD7475, AD7476, AD7477, AD7478, AD7466, AD7467,
> AD7468,
> - AD7495, AD7910, AD7920, AD7920 SPI analog to digital converters
> (ADC).
> + AD7495, AD7910, AD7920, AD7920, ADS7866, ADS7867, ADS7868 SPI analog
> + to digital converters (ADC).
The driver is for Analog Devices parts.
And these new parts are from TI.
I'm not sure what the policy is for mixing manufacturers in drivers.
But if that is allowed/fine, then maybe it would be a good idea to
highlight that these chips are from another manufacturer.
Or maybe rename the driver to a generic form to support both manufacturers ?
In any case, Jonathan will have to tell us what to do here :)
Thanks
Alex
>
> To compile this driver as a module, choose M here: the
> module will be called ad7476.
> diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c
> index 0549686b9ef8..108c18d64f7d 100644
> --- a/drivers/iio/adc/ad7476.c
> +++ b/drivers/iio/adc/ad7476.c
> @@ -59,6 +59,9 @@ enum ad7476_supported_device_ids {
> ID_ADC081S,
> ID_ADC101S,
> ID_ADC121S,
> + ID_ADS7866,
> + ID_ADS7867,
> + ID_ADS7868,
> };
>
> static irqreturn_t ad7476_trigger_handler(int irq, void *p)
> @@ -157,6 +160,8 @@ static int ad7476_read_raw(struct iio_dev *indio_dev,
> #define AD7940_CHAN(bits) _AD7476_CHAN((bits), 15 - (bits), \
> BIT(IIO_CHAN_INFO_RAW))
> #define AD7091R_CHAN(bits) _AD7476_CHAN((bits), 16 - (bits), 0)
> +#define ADS786X_CHAN(bits) _AD7476_CHAN((bits) + 4, 0, \
> + BIT(IIO_CHAN_INFO_RAW))
>
> static const struct ad7476_chip_info ad7476_chip_info_tbl[] = {
> [ID_AD7091R] = {
> @@ -209,6 +214,18 @@ static const struct ad7476_chip_info
> ad7476_chip_info_tbl[] = {
> .channel[0] = ADC081S_CHAN(12),
> .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> },
> + [ID_ADS7866] = {
> + .channel[0] = ADS786X_CHAN(8),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> + [ID_ADS7867] = {
> + .channel[0] = ADS786X_CHAN(10),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> + [ID_ADS7868] = {
> + .channel[0] = ADS786X_CHAN(12),
> + .channel[1] = IIO_CHAN_SOFT_TIMESTAMP(1),
> + },
> };
>
> static const struct iio_info ad7476_info = {
> @@ -314,6 +331,9 @@ static const struct spi_device_id ad7476_id[] = {
> {"adc081s", ID_ADC081S},
> {"adc101s", ID_ADC101S},
> {"adc121s", ID_ADC121S},
> + {"ads7866", ID_ADS7866},
> + {"ads7867", ID_ADS7867},
> + {"ads7868", ID_ADS7868},
> {}
> };
> MODULE_DEVICE_TABLE(spi, ad7476_id);
> --
> 2.20.1
>
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro wrote: > > Remove the checkpatch.pl check: > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? Hey, A bit curios about this one. Are you using this chip/driver ? Thing is: the part is nearing EOL, and it could be an idea to be marked for removal (since it's still in staging). But if there are users for this driver, it could be left around for a while. Thanks Alex > > Signed-off-by: Rodrigo Ribeiro > Signed-off-by: Rafael Tsuha > --- > This macro is not used anywhere. Should we just correct the > spelling or remove the macro? > > drivers/staging/iio/cdc/ad7152.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/cdc/ad7152.c > b/drivers/staging/iio/cdc/ad7152.c > index 25f51db..c9da6d4 100644 > --- a/drivers/staging/iio/cdc/ad7152.c > +++ b/drivers/staging/iio/cdc/ad7152.c > @@ -35,7 +35,7 @@ > #define AD7152_REG_CH2_GAIN_HIGH 12 > #define AD7152_REG_CH2_SETUP 14 > #define AD7152_REG_CFG 15 > -#define AD7152_REG_RESEVERD16 > +#define AD7152_REG_RESERVED16 > #define AD7152_REG_CAPDAC_POS 17 > #define AD7152_REG_CAPDAC_NEG 18 > #define AD7152_REG_CFG226 > -- > 2.7.4 >
Re: [PATCH 1/2] staging: iio: adc: ad7192: Add clock for external clock reference
On Fri, Jan 25, 2019 at 12:41 AM Stephen Boyd wrote: > > Quoting Jonathan Cameron (2018-12-16 02:07:41) > > Rob, Clk experts, questions for you below. > > > > Jonathan > > > > > > On Thu, 13 Dec 2018 17:39:22 -0800 > > Stephen Boyd wrote: > > > > > Quoting Jonathan Cameron (2018-12-08 07:29:54) > > > > On Thu, 6 Dec 2018 11:10:51 +0200 > > > > Mircea Caprioru wrote: > > > > > > > > > This patch adds a clock to the state structure of ad7192 for getting > > > > > the > > > > > external clock frequency. This modifications is in accordance with > > > > > clock > > > > > framework dt bindings documentation. > > > > > > > > > > Signed-off-by: Mircea Caprioru > > > > > > > > +cc Rob and the clk list for advise on how to do the binding for this > > > > one. > > > > > > > > It is basically 2 pins, you can put a clock in on one of them or connect > > > > a crystal across them. The driver has to set a register to say which is > > > > the case. > > > > > > > > Current proposal is two optional clocks (fall back to internal > > > > oscillator) > > > > but that doesn't seem to be commonly done, so I'm wondering if there > > > > is a 'standard' way to handle this sort of thing. > > > > > > > > > > I'm not sure I fully understand, but I think perhaps > > > assigned-clock-parents would work? Or does that not work because either > > > way some parent is assigned, either the crystal or the optional clk that > > > isn't a crystal? > > > > > My concern is they aren't really separate clock inputs. They are just > > different > > ways of providing the same fundamental clock. So I think we may want to > > just > > provide a single clock and have another dt binding to say what it is. > > > > So lots of ways we could do it, but I'm not sure what the right one to > > go with is! > > > > Sorry for getting back to this so late. Is the datasheet public for this > device? If so, any link to it? > Hey, Link is http://analog.com/ad7192 and that should resolve to the proper page, where the datasheet is available. [ General info: most [if not all] datasheets from Analog Devices can be found by concatenating "http://analog.com/ + "" ] But more directly, the link to the PDF is: https://www.analog.com/media/en/technical-documentation/data-sheets/AD7192.pdf Page 10 provides some description of the pins, page 20 the mode register for the clock, and page 32 a general description of the clock. If you search for MCLK1 or MCLK2 you should navigate pretty quick through the doc. The clock circuitry [the 2 pins] is common for all chips this driver supports [AD7190/2/3/5]. Thanks Alex > If it's two pins, and sometimes one pin is connected and other times two > pins are connected but a register needs to be set if the pins are > connected in one configuration or the other I would say your plan for a > DT property indicating how the pins are configured sounds good. Usually > the hardware can figure these things out itself so I find this sort of > odd, but if this is how it is then there's not much that we can do. > > It sounds like there aren't two different clk inputs to the device. > Given that information specifying two optional clks is incorrect because > there is only one 'slot' is the external clk source. >
Re: [PATCH] drivers: iio: industrialio-core: add check when kzalloc fails
On Thu, Jan 24, 2019 at 4:28 PM Bharath Vedartham wrote:
>
> add code to handle the case when kzalloc fails to allocate memory to dev
>
> Signed-off-by: Bharath Vedartham
> ---
> drivers/iio/industrialio-core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 4f5cd9f..93caa6b 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1475,6 +1475,8 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
> }
> dev_set_name(&dev->dev, "iio:device%d", dev->id);
> INIT_LIST_HEAD(&dev->buffer_list);
> + } else {
> + return NULL;
I'd argue that this is a bit redundant, because `dev` is NULL, the
return below (return dev) will also return NULL.
Alex
> }
>
> return dev;
> --
> 2.7.4
>
