[PATCH 3/7] staging: mt7621-dts: allow gnubee to reboot cleanly.
Since commit bb276262e88d ("mtd: spi-nor: only apply reset hacks to
broken hardware"), we need to mark the spi-nor as "broken" for reboot
to work.
Note that nothing is actually broken here. The hardware-watchdog in
the SoC isn't wired in a way that works, but then the board doesn't
claim to support a hardware watchdog - and the SPI certain isn't
"broken".
This causes an annoying warning on every boot, but that is better than
failing on ever reboot.
Signed-off-by: NeilBrown
---
drivers/staging/mt7621-dts/gbpc1.dts |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/mt7621-dts/gbpc1.dts
b/drivers/staging/mt7621-dts/gbpc1.dts
index d5b27e224b56..6a1699ce9455 100644
--- a/drivers/staging/mt7621-dts/gbpc1.dts
+++ b/drivers/staging/mt7621-dts/gbpc1.dts
@@ -72,6 +72,7 @@
compatible = "jedec,spi-nor";
reg = <0>;
spi-max-frequency = <5000>;
+ broken-flash-reset;
partition@0 {
label = "u-boot";
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/7] staging: mt7621-mmc: add missing header.
is sometimes included by other include files, and sometimes not, depending on config, particularly CONFIG_HIGHMEM. So include it explicitly rather than relying on implicit inclusion. Signed-off-by: NeilBrown --- drivers/staging/mt7621-mmc/sd.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c index 6320bf5a6dd5..631d1311f331 100644 --- a/drivers/staging/mt7621-mmc/sd.c +++ b/drivers/staging/mt7621-mmc/sd.c @@ -38,6 +38,7 @@ #include #include #include +#include #include #include ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/7] staging: mt7621-eth: set correct dma mask.
Since commit f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for
simple noncoherent platforms") changed MIPS dma handling, the eth
driver fails because the dma mask is set on the wrong 'struct device'.
Move the setting to the correct struct device.
Signed-off-by: NeilBrown
---
drivers/staging/mt7621-eth/mtk_eth_soc.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.c
b/drivers/staging/mt7621-eth/mtk_eth_soc.c
index 363d3c978e02..21a76a8ccc26 100644
--- a/drivers/staging/mt7621-eth/mtk_eth_soc.c
+++ b/drivers/staging/mt7621-eth/mtk_eth_soc.c
@@ -1689,6 +1689,8 @@ static int mtk_open(struct net_device *dev)
struct mtk_mac *mac = netdev_priv(dev);
struct mtk_eth *eth = mac->hw;
+ dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(32));
+
if (!atomic_read(>dma_refcnt)) {
int err = mtk_start_dma(eth);
@@ -2062,9 +2064,6 @@ static int mtk_probe(struct platform_device *pdev)
struct clk *sysclk;
int err;
- pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
- pdev->dev.dma_mask = >dev.coherent_dma_mask;
-
device_reset(>dev);
match = of_match_device(of_mtk_match, >dev);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/7] staging: mt7621-mmc: fix compile warnging: cmd_buf
cmd_buf is only used when MT6575_SD_DEBUG is defined.
So only declare it in that case.
Signed-off-by: NeilBrown
---
drivers/staging/mt7621-mmc/dbg.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/mt7621-mmc/dbg.c b/drivers/staging/mt7621-mmc/dbg.c
index 829d3d0e895e..eabe0595978b 100644
--- a/drivers/staging/mt7621-mmc/dbg.c
+++ b/drivers/staging/mt7621-mmc/dbg.c
@@ -51,7 +51,6 @@
#include "mt6575_sd.h"
#include
-static char cmd_buf[256];
/* for debug zone */
unsigned int sd_debug_zone[4] = {
@@ -62,6 +61,7 @@ unsigned int sd_debug_zone[4] = {
};
#if defined(MT6575_SD_DEBUG)
+static char cmd_buf[256];
/* for driver profile */
#define TICKS_ONE_MS (13000)
u32 gpt_enable;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/7] staging: mt7621-mmc: set correct dma mask.
Since commit f8c55dc6e828 ("MIPS: use generic dma noncoherent ops for
simple noncoherent platforms") changed MIPS dma handling, the mmc
driver fails because it doesn't have a dma mask is set.
So set the correct dma mask.
Signed-off-by: NeilBrown
---
drivers/staging/mt7621-mmc/sd.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 3d918e481bd8..6320bf5a6dd5 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -1673,7 +1673,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
host->mrq = NULL;
//init_MUTEX(>sem); /* we don't need to support multiple threads
access */
- mmc_dev(mmc)->dma_mask = NULL;
+ dma_coerce_mask_and_coherent(mmc_dev(mmc), DMA_BIT_MASK(32));
/* using dma_alloc_coherent*/ /* todo: using 1, for all 4 slots */
host->dma.gpd = dma_alloc_coherent(>dev,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/7] staging: mt7621-eth: fix compile warning.
This code generates a waring as PHY_GBIT_FEATURES is "long" but ->supported in "int". It looks likely that "PHY_1000BT_FEATURES" is the correct define to use - it is intended to be used with the ->features field. Signed-off-by: NeilBrown --- drivers/staging/mt7621-eth/mdio.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/mt7621-eth/mdio.c b/drivers/staging/mt7621-eth/mdio.c index ee851281b657..5fea6a447eed 100644 --- a/drivers/staging/mt7621-eth/mdio.c +++ b/drivers/staging/mt7621-eth/mdio.c @@ -89,7 +89,7 @@ int mtk_connect_phy_node(struct mtk_eth *eth, struct mtk_mac *mac, return -ENODEV; } - phydev->supported &= PHY_GBIT_FEATURES; + phydev->supported &= PHY_1000BT_FEATURES; phydev->advertising = phydev->supported; dev_info(eth->dev, ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/7] staging: mt7621-dts: avoid warnings in pinctrl definitions
The device-tree checking code sees node names "i2c" and "spi" in the
pinctrl definition and thinks these are defining i2c or spi devices,
and complains that they look wrong.
So add a '0' to the end of each name (much like "uart" and "rgmii"
have numbers at the end) to avoid the warning.
Signed-off-by: NeilBrown
---
drivers/staging/mt7621-dts/mt7621.dtsi | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi
b/drivers/staging/mt7621-dts/mt7621.dtsi
index ebf6667bbc54..71f069d59ad8 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -202,15 +202,15 @@
state_default: pinctrl0 {
};
- i2c_pins: i2c {
- i2c {
+ i2c_pins: i2c0 {
+ i2c0 {
group = "i2c";
function = "i2c";
};
};
- spi_pins: spi {
- spi {
+ spi_pins: spi0 {
+ spi0 {
group = "spi";
function = "spi";
};
@@ -251,21 +251,21 @@
};
};
- mdio_pins: mdio {
- mdio {
+ mdio_pins: mdio0 {
+ mdio0 {
group = "mdio";
function = "mdio";
};
};
- pcie_pins: pcie {
- pcie {
+ pcie_pins: pcie0 {
+ pcie0 {
group = "pcie";
function = "pcie rst";
};
};
- nand_pins: nand {
+ nand_pins: nand0 {
spi-nand {
group = "spi";
function = "nand1";
@@ -277,8 +277,8 @@
};
};
- sdhci_pins: sdhci {
- sdhci {
+ sdhci_pins: sdhci0 {
+ sdhci0 {
group = "sdhci";
function = "sdhci";
};
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/7] various fixes for mt7621-* in staging.
These fixes, together with some that others have posted recently, allow my to once again compile mainline linux for my gnubee-pc1, and have it work. Thanks, NeilBrown --- NeilBrown (7): staging: mt7621-eth: set correct dma mask. staging: mt7621-mmc: set correct dma mask. staging: mt7621-dts: allow gnubee to reboot cleanly. staging: mt7621-eth: fix compile warning. staging: mt7621-mmc: add missing header. staging: mt7621-mmc: fix compile warnging: cmd_buf staging: mt7621-dts: avoid warnings in pinctrl definitions drivers/staging/mt7621-dts/gbpc1.dts |1 + drivers/staging/mt7621-dts/mt7621.dtsi | 22 +++--- drivers/staging/mt7621-eth/mdio.c|2 +- drivers/staging/mt7621-eth/mtk_eth_soc.c |5 ++--- drivers/staging/mt7621-mmc/dbg.c |2 +- drivers/staging/mt7621-mmc/sd.c |3 ++- 6 files changed, 18 insertions(+), 17 deletions(-) -- Signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 0/2] staging: mt7621-spi: drop broken SPI modes
On Thu, Dec 06 2018, Chuanhong Guo wrote: > This SPI controller seems to be tested on SPI flash only and SPI mode > 1/2/3 and full-duplex mode is broken. (Details are in commit messages > for the two patches.) > This patchset drops those broken modes. > > Changes since v2: > Send the two commit as a patchset > fix code style complained by checkpatch.pl > > Chuanhong Guo (2): > staging: mt7621-spi: drop the broken full-duplex mode > staging: mt7621-spi: drop support for SPI mode 1/2/3 Looks great! Reviewed-by: NeilBrown Thanks, NeilBrown > > drivers/staging/mt7621-spi/spi-mt7621.c | 147 > 1 file changed, 24 insertions(+), 123 deletions(-) > > -- > 2.19.1 signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] Add devicetree support for ad5933
On Sat, Dec 08, 2018 at 04:56:45PM -0200, Marcelo Schmitt wrote: > Parts of this work came from contributions of Alexandru Ardelean and > Dragos Bogdan, I and Gabriel would like to thank for the insights > provided by their previous patches. Maybe it would be the case to add > them as co-authors of this patch set. That's what the Co-developed-by: tag is for, please use it :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] Add devicetree support for ad5933
Parts of this work came from contributions of Alexandru Ardelean and Dragos Bogdan, I and Gabriel would like to thank for the insights provided by their previous patches. Maybe it would be the case to add them as co-authors of this patch set. We also wanted to thank Jhonatan Cameron for giving us the pieces of advice needed for this work. Thanks, Marcelo Em sáb, 8 de dez de 2018 às 16:18, Marcelo Schmitt escreveu: > > This series of patches change voltage regulator error handling for the > ad5933. > It also add an option to specify external clock reference using a clock > framework and remove the old platform data structure. > Finally it adds binding documentation for devicetree. > > Marcelo Schmitt (3): > staging: iio: ad5933: change regulator binging for vref > staging: iio: ad5933: use clock framework for clock reference > staging: iio: ad5933: add binding doc for ad5933 > > .../iio/impedance-analyzer/ad5933.txt | 26 + > .../staging/iio/impedance-analyzer/ad5933.c | 57 +-- > 2 files changed, 54 insertions(+), 29 deletions(-) > create mode 100644 > Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt > > -- > 2.17.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: iio: ad5933: add binding doc for ad5933
Add a devicetree documentation for the ad5933 and ad5934 impedance
converter, network analyzer.
Signed-off-by: Marcelo Schmitt
Signed-off-by: Gabriel Capella
Co-Developed-by: Gabriel Capella
---
.../iio/impedance-analyzer/ad5933.txt | 26 +++
1 file changed, 26 insertions(+)
create mode 100644
Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
diff --git
a/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
b/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
new file mode 100644
index ..5ff38728ff91
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
@@ -0,0 +1,26 @@
+Analog Devices AD5933/AD5934 Impedance Converter, Network Analyzer
+
+https://www.analog.com/media/en/technical-documentation/data-sheets/AD5933.pdf
+https://www.analog.com/media/en/technical-documentation/data-sheets/AD5934.pdf
+
+Required properties:
+ - compatible : should be one of
+ "adi,ad5933"
+ "adi,ad5934"
+ - reg : the I2C address.
+ - vdd-supply : The regulator supply for DVDD, AVDD1 and AVDD2 when they
+ are connected together.
+
+Optional properties:
+- clocks : external clock reference.
+- clock-names : must be "mclk" if clocks is set.
+
+Example for a I2C device node:
+
+ impedance-analyzer@0d {
+ compatible = "adi,adxl345";
+ reg = <0x0d>;
+ vdd-supply = <_supply>;
+ clocks = <_clk>;
+ clock-names = "mclk";
+ };
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: iio: ad5933: change regulator binging for vref
Set a single voltage regulator for all voltage references.
Remove voltage reference value from default platafrom data struct.
Signed-off-by: Marcelo Schmitt
Signed-off-by: Gabriel Capella
Co-Developed-by: Gabriel Capella
---
drivers/staging/iio/impedance-analyzer/ad5933.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 9e52384f5370..730bc397a26b 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -91,7 +91,6 @@
struct ad5933_platform_data {
unsigned long ext_clk_hz;
- unsigned short vref_mv;
};
struct ad5933_state {
@@ -113,7 +112,6 @@ struct ad5933_state {
};
static struct ad5933_platform_data ad5933_default_pdata = {
- .vref_mv = 3300,
};
#define AD5933_CHANNEL(_type, _extend_name, _info_mask_separate, _address, \
@@ -691,7 +689,7 @@ static void ad5933_work(struct work_struct *work)
static int ad5933_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
- int ret, voltage_uv = 0;
+ int ret;
struct ad5933_platform_data *pdata = dev_get_platdata(>dev);
struct ad5933_state *st;
struct iio_dev *indio_dev;
@@ -718,12 +716,12 @@ static int ad5933_probe(struct i2c_client *client,
dev_err(>dev, "Failed to enable specified VDD
supply\n");
return ret;
}
- voltage_uv = regulator_get_voltage(st->reg);
+ ret = regulator_get_voltage(st->reg);
+
+ if (ret < 0)
+ goto error_disable_reg;
- if (voltage_uv)
- st->vref_mv = voltage_uv / 1000;
- else
- st->vref_mv = pdata->vref_mv;
+ st->vref_mv = ret / 1000;
if (pdata->ext_clk_hz) {
st->mclk_hz = pdata->ext_clk_hz;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: iio: ad5933: use clock framework for clock reference
Add the option to specify the external clock (MCLK) using the clock
framework.
Also remove the old platform data structure.
Signed-off-by: Marcelo Schmitt
Signed-off-by: Gabriel Capella
Co-Developed-by: Gabriel Capella
---
.../staging/iio/impedance-analyzer/ad5933.c | 43 ++-
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 730bc397a26b..3134295f014f 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -82,20 +83,10 @@
#define AD5933_POLL_TIME_ms10
#define AD5933_INIT_EXCITATION_TIME_ms 100
-/**
- * struct ad5933_platform_data - platform specific data
- * @ext_clk_hz:the external clock frequency in Hz, if not set
- * the driver uses the internal clock (16.776 MHz)
- * @vref_mv: the external reference voltage in millivolt
- */
-
-struct ad5933_platform_data {
- unsigned long ext_clk_hz;
-};
-
struct ad5933_state {
struct i2c_client *client;
struct regulator*reg;
+ struct clk *mclk;
struct delayed_work work;
struct mutexlock; /* Protect sensor state */
unsigned long mclk_hz;
@@ -111,9 +102,6 @@ struct ad5933_state {
unsigned intpoll_time_jiffies;
};
-static struct ad5933_platform_data ad5933_default_pdata = {
-};
-
#define AD5933_CHANNEL(_type, _extend_name, _info_mask_separate, _address, \
_scan_index, _realbits) { \
.type = (_type), \
@@ -690,9 +678,9 @@ static int ad5933_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
int ret;
- struct ad5933_platform_data *pdata = dev_get_platdata(>dev);
struct ad5933_state *st;
struct iio_dev *indio_dev;
+ unsigned long ext_clk_hz = 0;
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
@@ -704,9 +692,6 @@ static int ad5933_probe(struct i2c_client *client,
mutex_init(>lock);
- if (!pdata)
- pdata = _default_pdata;
-
st->reg = devm_regulator_get(>dev, "vdd");
if (IS_ERR(st->reg))
return PTR_ERR(st->reg);
@@ -723,8 +708,21 @@ static int ad5933_probe(struct i2c_client *client,
st->vref_mv = ret / 1000;
- if (pdata->ext_clk_hz) {
- st->mclk_hz = pdata->ext_clk_hz;
+ st->mclk = devm_clk_get(>dev, "mclk");
+ if (IS_ERR(st->mclk) && PTR_ERR(st->mclk) != -ENOENT) {
+ ret = PTR_ERR(st->mclk);
+ goto error_disable_reg;
+ }
+
+ if (!IS_ERR(st->mclk)) {
+ ret = clk_prepare_enable(st->mclk);
+ if (ret < 0)
+ goto error_disable_reg;
+ ext_clk_hz = clk_get_rate(st->mclk);
+ }
+
+ if (ext_clk_hz) {
+ st->mclk_hz = ext_clk_hz;
st->ctrl_lb = AD5933_CTRL_EXT_SYSCLK;
} else {
st->mclk_hz = AD5933_INT_OSC_FREQ_Hz;
@@ -744,7 +742,7 @@ static int ad5933_probe(struct i2c_client *client,
ret = ad5933_register_ring_funcs_and_init(indio_dev);
if (ret)
- goto error_disable_reg;
+ goto error_disable_mclk;
ret = ad5933_setup(st);
if (ret)
@@ -758,6 +756,8 @@ static int ad5933_probe(struct i2c_client *client,
error_unreg_ring:
iio_kfifo_free(indio_dev->buffer);
+error_disable_mclk:
+ clk_disable_unprepare(st->mclk);
error_disable_reg:
regulator_disable(st->reg);
@@ -772,6 +772,7 @@ static int ad5933_remove(struct i2c_client *client)
iio_device_unregister(indio_dev);
iio_kfifo_free(indio_dev->buffer);
regulator_disable(st->reg);
+ clk_disable_unprepare(st->mclk);
return 0;
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] Add devicetree support for ad5933
This series of patches change voltage regulator error handling for the ad5933. It also add an option to specify external clock reference using a clock framework and remove the old platform data structure. Finally it adds binding documentation for devicetree. Marcelo Schmitt (3): staging: iio: ad5933: change regulator binging for vref staging: iio: ad5933: use clock framework for clock reference staging: iio: ad5933: add binding doc for ad5933 .../iio/impedance-analyzer/ad5933.txt | 26 + .../staging/iio/impedance-analyzer/ad5933.c | 57 +-- 2 files changed, 54 insertions(+), 29 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/3] Staging: iio: adt7316: Move interrupt related code
On Sat, 2018-12-08 at 16:12 +, Jonathan Cameron wrote:
> On Sat, 8 Dec 2018 20:46:37 +0530
> Shreeya Patel wrote:
>
> > There is a function adt7316_irq_setup() where irq_type is being
> > set. It would be good to move devm_request_threaded_irq() function
> > and assignment of chip->config1 in adt7316_irq_setup() to unclutter
> > the code in probe function.
> >
> > Signed-off-by: Shreeya Patel
>
> As commented below, this didn't end up as tidy as it might have been.
> It would I think have been simpler before patch 1 or just merged with
> it.
>
As I was introducing a new function named "adt7316_setup_irq" so I
thought patch 1 should come first because we are setting up the
irq_type there.
But yes, this made the code complex to review.
I didn't merge both patches because both the patches were having
different changes. If I would have done that then there was a
possibility where someone would have said to split the patches.
> Anyhow, I might combine the two whilst applying. However before I do
> that I'd like to leave this on list for a few days to let Alex
> or others have time for another look before I apply it.
>
It's ok, I'll merge both patches and send as a v4 to you. I'll send it
after 3-4 days so we can get other reviews by that time if there are
any to come.
My vacation has started so I'll work faster now.
> All heading in the right direction!
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/staging/iio/addac/adt7316.c | 34 ++---
> >
> > 1 file changed, 17 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/staging/iio/addac/adt7316.c
> > b/drivers/staging/iio/addac/adt7316.c
> > index 86b2c3d53588..97dd48153293 100644
> > --- a/drivers/staging/iio/addac/adt7316.c
> > +++ b/drivers/staging/iio/addac/adt7316.c
> > @@ -1807,11 +1807,12 @@ static irqreturn_t
> > adt7316_event_handler(int irq, void *private)
> > return IRQ_HANDLED;
> > }
> >
> > -static int adt7316_setup_irq(struct device *dev, int irq)
> > +static int adt7316_setup_irq(struct iio_dev *indio_dev)
>
> Hmm. This has ended up a lot more complex than ideal due
> to the effective two layers of rework.
>
> I would either have done patches 1 and 2 as one patch or
> reordered them so the rework preceded the change
> to DT. It's not that important but it would have lead
> to code that was easier to review.
>
>
> > {
> > - int irq_type;
> > + struct adt7316_chip_info *chip = iio_priv(indio_dev);
> > + int irq_type, ret;
> >
> > - irq_type = irqd_get_trigger_type(irq_get_irq_data(irq));
> > + irq_type = irqd_get_trigger_type(irq_get_irq_data(chip-
> > >bus.irq));
> >
> > switch (irq_type) {
> > case IRQF_TRIGGER_HIGH:
> > @@ -1821,13 +1822,23 @@ static int adt7316_setup_irq(struct device
> > *dev, int irq)
> > case IRQF_TRIGGER_FALLING:
> > break;
> > default:
> > - dev_info(dev, "mode %d unsupported, using
> > IRQF_TRIGGER_LOW\n",
> > + dev_info(_dev->dev, "mode %d unsupported,
> > using IRQF_TRIGGER_LOW\n",
> > irq_type);
> > irq_type = IRQF_TRIGGER_LOW;
> > break;
> > }
> >
> > - return irq_type;
> > + ret = devm_request_threaded_irq(_dev->dev, chip-
> > >bus.irq,
> > + NULL,
> > adt7316_event_handler,
> > + irq_type | IRQF_ONESHOT,
> > + indio_dev->name,
> > indio_dev);
> > + if (ret)
> > + return ret;
> > +
> > + if (irq_type & IRQF_TRIGGER_HIGH)
> > + chip->config1 |= ADT7316_INT_POLARITY;
> > +
> > + return ret;
> > }
> >
> > /*
> > @@ -2124,7 +2135,6 @@ int adt7316_probe(struct device *dev, struct
> > adt7316_bus *bus,
> > {
> > struct adt7316_chip_info *chip;
> > struct iio_dev *indio_dev;
> > - int irq_type;
> > int ret = 0;
> >
> > indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
> > @@ -2168,19 +2178,9 @@ int adt7316_probe(struct device *dev, struct
> > adt7316_bus *bus,
> > indio_dev->modes = INDIO_DIRECT_MODE;
> >
> > if (chip->bus.irq > 0) {
> > - irq_type = adt7316_setup_irq(dev, chip->bus.irq);
> > -
> > - ret = devm_request_threaded_irq(dev, chip-
> > >bus.irq,
> > - NULL,
> > - adt7316_event_hand
> > ler,
> > - irq_type |
> > IRQF_ONESHOT,
> > - indio_dev->name,
> > - indio_dev);
> > + ret = adt7316_setup_irq(indio_dev);
> > if (ret)
> > return ret;
> > -
> > - if (irq_type & IRQF_TRIGGER_HIGH)
> > - chip->config1 |= ADT7316_INT_POLARITY;
> > }
> >
> > ret = chip->bus.write(chip->bus.client, ADT7316_CONFIG1,
> > chip->config1);
>
>
Re: [PATCH v3 2/3] Staging: iio: adt7316: Move interrupt related code
On Sat, 8 Dec 2018 20:46:37 +0530
Shreeya Patel wrote:
> There is a function adt7316_irq_setup() where irq_type is being
> set. It would be good to move devm_request_threaded_irq() function
> and assignment of chip->config1 in adt7316_irq_setup() to unclutter
> the code in probe function.
>
> Signed-off-by: Shreeya Patel
As commented below, this didn't end up as tidy as it might have been.
It would I think have been simpler before patch 1 or just merged with it.
Anyhow, I might combine the two whilst applying. However before I do
that I'd like to leave this on list for a few days to let Alex
or others have time for another look before I apply it.
All heading in the right direction!
Thanks,
Jonathan
> ---
> drivers/staging/iio/addac/adt7316.c | 34 ++---
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/iio/addac/adt7316.c
> b/drivers/staging/iio/addac/adt7316.c
> index 86b2c3d53588..97dd48153293 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -1807,11 +1807,12 @@ static irqreturn_t adt7316_event_handler(int irq,
> void *private)
> return IRQ_HANDLED;
> }
>
> -static int adt7316_setup_irq(struct device *dev, int irq)
> +static int adt7316_setup_irq(struct iio_dev *indio_dev)
Hmm. This has ended up a lot more complex than ideal due
to the effective two layers of rework.
I would either have done patches 1 and 2 as one patch or
reordered them so the rework preceded the change
to DT. It's not that important but it would have lead
to code that was easier to review.
> {
> - int irq_type;
> + struct adt7316_chip_info *chip = iio_priv(indio_dev);
> + int irq_type, ret;
>
> - irq_type = irqd_get_trigger_type(irq_get_irq_data(irq));
> + irq_type = irqd_get_trigger_type(irq_get_irq_data(chip->bus.irq));
>
> switch (irq_type) {
> case IRQF_TRIGGER_HIGH:
> @@ -1821,13 +1822,23 @@ static int adt7316_setup_irq(struct device *dev, int
> irq)
> case IRQF_TRIGGER_FALLING:
> break;
> default:
> - dev_info(dev, "mode %d unsupported, using IRQF_TRIGGER_LOW\n",
> + dev_info(_dev->dev, "mode %d unsupported, using
> IRQF_TRIGGER_LOW\n",
>irq_type);
> irq_type = IRQF_TRIGGER_LOW;
> break;
> }
>
> - return irq_type;
> + ret = devm_request_threaded_irq(_dev->dev, chip->bus.irq,
> + NULL, adt7316_event_handler,
> + irq_type | IRQF_ONESHOT,
> + indio_dev->name, indio_dev);
> + if (ret)
> + return ret;
> +
> + if (irq_type & IRQF_TRIGGER_HIGH)
> + chip->config1 |= ADT7316_INT_POLARITY;
> +
> + return ret;
> }
>
> /*
> @@ -2124,7 +2135,6 @@ int adt7316_probe(struct device *dev, struct
> adt7316_bus *bus,
> {
> struct adt7316_chip_info *chip;
> struct iio_dev *indio_dev;
> - int irq_type;
> int ret = 0;
>
> indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
> @@ -2168,19 +2178,9 @@ int adt7316_probe(struct device *dev, struct
> adt7316_bus *bus,
> indio_dev->modes = INDIO_DIRECT_MODE;
>
> if (chip->bus.irq > 0) {
> - irq_type = adt7316_setup_irq(dev, chip->bus.irq);
> -
> - ret = devm_request_threaded_irq(dev, chip->bus.irq,
> - NULL,
> - adt7316_event_handler,
> - irq_type | IRQF_ONESHOT,
> - indio_dev->name,
> - indio_dev);
> + ret = adt7316_setup_irq(indio_dev);
> if (ret)
> return ret;
> -
> - if (irq_type & IRQF_TRIGGER_HIGH)
> - chip->config1 |= ADT7316_INT_POLARITY;
> }
>
> ret = chip->bus.write(chip->bus.client, ADT7316_CONFIG1, chip->config1);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] iio: dac: ad5686: fix bit shift read register
On Thu, 6 Dec 2018 15:53:15 +0200
Mircea Caprioru wrote:
> This patch solves the register readback issue with the bit shift. When the
> dac resolution was lower than the register size (ex. 12 bits out of 16
> bits) the readback value was not shifted with the difference in bits and
> the value was higher. Also a mask is applied on the read value in order to
> get the value relative to the actual bit size.
>
> Fixes: 0357e488b8 ("iio:dac:ad5686: Refactor the driver")
> Signed-off-by: Mircea Caprioru
This seems obvious, but a little strange we've not noticed it
before.Ah well. Given the time in the cycle I'm going to
take it for the next merge window rather than rushing it it.
Applied to the togreg branch of iio.git and pushed out as
testing for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/dac/ad5686.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index 54ff76b93366..a332b93ca2c4 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
> @@ -128,7 +128,8 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
> mutex_unlock(_dev->mlock);
> if (ret < 0)
> return ret;
> - *val = ret;
> + *val = (ret >> chan->scan_type.shift) &
> + GENMASK(chan->scan_type.realbits - 1, 0);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SCALE:
> *val = st->vref_mv;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] iio:dac:ad5686: Add AD5310R support
On Thu, 6 Dec 2018 15:38:30 +0200
Mircea Caprioru wrote:
> From: Stefan Popa
>
> The AD5310R is a single channel DAC with 10-bit precision, which is
> part of the same family as AD5311R, except that it uses the spi interface
> instead of i2c. The device has a built-in 2.5V reference which is enabled
> by default.
>
> Another important difference is that the SPI write command operation is
> 16 bits long. The first four bits represent the command, while the
> remaining 12 bits are for data. In the control reg, DB9 and DB10 are used
> for power-down modes, while DB8 is the REF bit. In order to accommodate
> this change, a new regmap type was defined and checked accordingly.
>
> Because AD5310R does not have a readback register, the read_raw operation
> will return "Operation is not supported".
>
> Datasheet:
> Link:
> http://www.analog.com/media/en/technical-documentation/data-sheets/AD5310R_5311R.pdf
>
> Signed-off-by: Stefan Popa
> Signed-off-by: Mircea Caprioru
A few comments inline, but mostly just me being fussy about patch presentation
so applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/dac/ad5686-spi.c | 21 ++---
> drivers/iio/dac/ad5686.c | 16
> drivers/iio/dac/ad5686.h | 7 +++
> 3 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
> index 1df9143f55e9..665fa6bd9ced 100644
> --- a/drivers/iio/dac/ad5686-spi.c
> +++ b/drivers/iio/dac/ad5686-spi.c
> @@ -19,6 +19,12 @@ static int ad5686_spi_write(struct ad5686_state *st,
> u8 tx_len, *buf;
>
> switch (st->chip_info->regmap_type) {
> + case AD5310_REGMAP:
> + st->data[0].d16 = cpu_to_be16(AD5310_CMD(cmd) |
> + val);
> + buf = >data[0].d8[0];
> + tx_len = 2;
> + break;
> case AD5683_REGMAP:
> st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> AD5683_DATA(val));
> @@ -56,10 +62,18 @@ static int ad5686_spi_read(struct ad5686_state *st, u8
> addr)
> u8 cmd = 0;
> int ret;
>
> - if (st->chip_info->regmap_type == AD5686_REGMAP)
> - cmd = AD5686_CMD_READBACK_ENABLE;
> - else if (st->chip_info->regmap_type == AD5683_REGMAP)
> + switch (st->chip_info->regmap_type) {
This is fine, though I'd prefer it done really as rework
patch without the new device support followed by adding the support
later in the series. I prefer to have to do minimum thinking
whilst reviewing :)
> + case AD5310_REGMAP:
> + return -ENOTSUPP;
> + case AD5683_REGMAP:
> cmd = AD5686_CMD_READBACK_ENABLE_V2;
> + break;
> + case AD5686_REGMAP:
> + cmd = AD5686_CMD_READBACK_ENABLE;
> + break;
> + default:
> + return -EINVAL;
> + }
>
> st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
> AD5686_ADDR(addr));
> @@ -86,6 +100,7 @@ static int ad5686_spi_remove(struct spi_device *spi)
> }
>
> static const struct spi_device_id ad5686_spi_id[] = {
> + {"ad5310r", ID_AD5310R},
> {"ad5672r", ID_AD5672R},
> {"ad5676", ID_AD5676},
> {"ad5676r", ID_AD5676R},
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index 0e134b13967a..54ff76b93366 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
> @@ -83,6 +83,10 @@ static ssize_t ad5686_write_dac_powerdown(struct iio_dev
> *indio_dev,
> st->pwr_down_mask &= ~(0x3 << (chan->channel * 2));
>
> switch (st->chip_info->regmap_type) {
> + case AD5310_REGMAP:
> + shift = 9;
> + ref_bit_msk = AD5310_REF_BIT_MSK;
> + break;
> case AD5683_REGMAP:
> shift = 13;
> ref_bit_msk = AD5683_REF_BIT_MSK;
> @@ -221,6 +225,7 @@ static struct iio_chan_spec name[] = {
> \
> AD5868_CHANNEL(7, 7, bits, _shift), \
> }
>
> +DECLARE_AD5693_CHANNELS(ad5310r_channels, 10, 2);
> DECLARE_AD5693_CHANNELS(ad5311r_channels, 10, 6);
> DECLARE_AD5676_CHANNELS(ad5672_channels, 12, 4);
> DECLARE_AD5676_CHANNELS(ad5676_channels, 16, 0);
> @@ -232,6 +237,12 @@ DECLARE_AD5693_CHANNELS(ad5692r_channels, 14, 2);
> DECLARE_AD5693_CHANNELS(ad5691r_channels, 12, 4);
>
> static const struct ad5686_chip_info ad5686_chip_info_tbl[] = {
> + [ID_AD5310R] = {
> + .channels = ad5310r_channels,
> + .int_vref_mv = 2500,
> + .num_channels = 1,
> + .regmap_type = AD5310_REGMAP,
> + },
> [ID_AD5311R] = {
> .channels = ad5311r_channels,
> .int_vref_mv = 2500,
> @@ -419,6 +430,11 @@ int ad5686_probe(struct device *dev,
>
Re: [PATCH] Revert "Staging: iio: adt7316: Add an extra check for 'ret' equals to 0"
On Sat, 2018-12-08 at 11:17 +, Jonathan Cameron wrote:
> On Sat, 08 Dec 2018 00:07:21 +0530
> Shreeya Patel wrote:
>
> > On Thu, 2018-12-06 at 15:40 +0300, Dan Carpenter wrote:
> > > On Wed, Dec 05, 2018 at 02:59:53PM -0700, Jeremy Fertic wrote:
> > > > On Thu, Dec 06, 2018 at 01:25:55AM +0530, Shreeya Patel
> > > > wrote:
> > > > > On Tue, 2018-12-04 at 18:49 -0700, Jeremy Fertic wrote:
> > > > > > This reverts commit
> > > > > > 00426e99789357dbff7e719a092ce36a3ce49d94.
> > > > > >
> > > > > > i2c_smbus_read_byte() returns 0 when a byte with the value
> > > > > > 0 is
> > > > > > read
> > > > > > from
> > > > > > the device. This is a valid read so revert the check for 0.
> > > > > >
> > > > > > Signed-off-by: Jeremy Fertic
> > > > > > ---
> > > > >
> > > > > Hi Jeremy,
> > > > >
> > > > > As per my understanding, 0 value indicates no error but no
> > > > > data
> > > > > read.
> > > > > Then how can this be a valid case?
> > > > >
> > > > > Can you please make me understand that how can we consider
> > > > > this
> > > > > as a
> > > > > valid case even when no data has been read?
> > >
> > > It's not reading no data. It's reading one byte of data and
> > > returning
> > > it.
> > >
> > > > >
> > > > >
> > > > > Thanks
> > > >
> > > > I'm not sure I understand why the value 0 would indicate no
> > > > data
> > > > read.
> > > > Doesn't that just mean a byte was read with the value 0.
> > >
> > > Yes. It does mean that. Please don't ask rhetorical
> > > questions... :(
> > > This list is full of people who can't resist answering every
> > > question.
> > >
> > > > For instance, if the input to the adc is 0V. Can you point me
> > > > to
> > > > where
> > > > you're seeing that this would indicate no data read?
> > >
> > > drivers/i2c/i2c-core-smbus.c
> > > 88 /**
> > > 89 * i2c_smbus_read_byte - SMBus "receive byte" protocol
> > > 90 * @client: Handle to slave device
> > > 91 *
> > > 92 * This executes the SMBus "receive byte" protocol,
> > > returning
> > > negative errno
> > > 93 * else the byte received from the device.
> > > 94 */
> > > 95 s32 i2c_smbus_read_byte(const struct i2c_client *client)
> > > 96 {
> > > 97 union i2c_smbus_data data;
> > > 98 int status;
> > > 99
> > >100 status = i2c_smbus_xfer(client->adapter,
> > > client-
> > > > addr, client->flags,
> > >
> > >101 I2C_SMBUS_READ, 0,
> > >102 I2C_SMBUS_BYTE, );
> > >103 return (status < 0) ? status : data.byte;
> > > ^
> > >104 }
> > >105 EXPORT_SYMBOL(i2c_smbus_read_byte);
> >
> > Even I had sent the same code to Jonathan and we had a discussion
> > on
> > this.
> > I asked him that this code clearly shows that there is an error
> > condition only when status < 0 then why do we need a check for
> > status =
> > 0.
> >
> > Then he explained me that 0 isn't an error. The issue is the
> > silliness
> > of the i2c interface.
> >
> > Pretty much every other bus returns an error (negative) if less
> > data is
> > received than expected. Most i2c
> > bus master's do as well but in theory it can return 0 to indicate
> > no
> > error but no data read (which doesn't make any sense)
> >
> > 0 doesn't ever happen in reality but it should be handled for
> > correctness though.
> >
> > So we should wait for what Jonathan has to say on this :)
>
> Yup, I was being an idiot. Sorry about that! For some reason I'd
> gotten it into my head that the particular function we were talking
> about was i2c_master_send which does indeed do as discussed above.
>
> Apologies for misleading you on this. Definitely a proper idiot
> moment of me not reading what the code actually was properly, even
> when you questioned what I was going on about.
It was not your mistake!
There was a confusion because of delay in replying to you from my side.
So it was just the case of human error :)
>
> Thanks to Jeremy for catching this one.
>
> Applied to the togreg branch of iio.git and pushed out as testing for
> the autobuilders to play with it.
>
> Jonathan
>
> >
> > Thanks
> >
> > > You are right. Commit 00426e997893 ("Staging: iio: adt7316: Add
> > > an
> > > extra check for 'ret' equals to 0") needs to be reverted...
> > >
> > > regards,
> > > dan carpenter
> > >
> > >
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: iio: adc: ad7192: Add clock output option
On Thu, 6 Dec 2018 11:10:52 +0200
Mircea Caprioru wrote:
> When using the internal clock the device has an option in which the clock
> output is available on MCLK2 pin. This patch adds a dt binding for enabling
> this property.
>
> Signed-off-by: Mircea Caprioru
I'd rather we looked at getting this out of staging, with proper binding
docs before we add too many new features, but I suppose this is small
enough that it isn't too much of a problem other that we are defining
a binding that might not go down well long term.
If nothing else we should be supporting this as a clock source
to allow it for example to be looped into another identical device
as the input.
Here we are putting it out on a pin with no 'consumers'.
Jonathan
> ---
> drivers/staging/iio/adc/ad7192.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/adc/ad7192.c
> b/drivers/staging/iio/adc/ad7192.c
> index 8a4e6ede42b3..a11c8a82bb7c 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -611,6 +611,10 @@ static const struct iio_chan_spec ad7193_channels[] = {
> static int ad7192_clock_select(struct spi_device *spi, struct ad7192_state
> *st)
> {
> int ret;
> + bool clock_out_en;
> +
> + clock_out_en = of_property_read_bool(spi->dev.of_node,
> + "adi,int-clock-output-enable");
>
> st->clock_sel = AD7192_CLK_EXT_MCLK2;
> st->mclk = devm_clk_get(>dev, "clk");
> @@ -626,7 +630,10 @@ static int ad7192_clock_select(struct spi_device *spi,
> struct ad7192_state *st)
> return PTR_ERR(st->mclk);
>
> /* use internal clock */
> - st->clock_sel = AD7192_CLK_INT;
> + if (!clock_out_en)
> + st->clock_sel = AD7192_CLK_INT;
> + else
> + st->clock_sel = AD7192_CLK_INT_CO;
> st->fclk = AD7192_INT_FREQ_MHZ;
> }
> }
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: iio: adc: ad7192: Add clock for external clock reference
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.
Thanks,
Jonathan
> ---
> drivers/staging/iio/adc/ad7192.c | 74 +---
> drivers/staging/iio/adc/ad7192.h | 2 -
> 2 files changed, 50 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7192.c
> b/drivers/staging/iio/adc/ad7192.c
> index acdbc07fd259..8a4e6ede42b3 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -6,6 +6,7 @@
> * Licensed under the GPL-2.
> */
>
> +#include
> #include
> #include
> #include
> @@ -156,8 +157,9 @@
> struct ad7192_state {
> struct regulator*avdd;
> struct regulator*dvdd;
> + struct clk *mclk;
> u16 int_vref_mv;
> - u32 mclk;
> + u32 fclk;
> u32 f_order;
> u32 mode;
> u32 conf;
> @@ -165,6 +167,7 @@ struct ad7192_state {
> u8 gpocon;
> u8 devid;
> struct mutexlock; /* protect sensor state */
> + u8 clock_sel;
>
> struct ad_sigma_delta sd;
> };
> @@ -250,28 +253,8 @@ static int ad7192_setup(struct ad7192_state *st,
> dev_warn(>sd.spi->dev, "device ID query failed (0x%X)\n",
>id);
>
> - switch (pdata->clock_source_sel) {
> - case AD7192_CLK_INT:
> - case AD7192_CLK_INT_CO:
> - st->mclk = AD7192_INT_FREQ_MHZ;
> - break;
> - case AD7192_CLK_EXT_MCLK1_2:
> - case AD7192_CLK_EXT_MCLK2:
> - if (ad7192_valid_external_frequency(pdata->ext_clk_hz)) {
> - st->mclk = pdata->ext_clk_hz;
> - break;
> - }
> - dev_err(>sd.spi->dev, "Invalid frequency setting %u\n",
> - pdata->ext_clk_hz);
> - ret = -EINVAL;
> - goto out;
> - default:
> - ret = -EINVAL;
> - goto out;
> - }
> -
> st->mode = AD7192_MODE_SEL(AD7192_MODE_IDLE) |
> - AD7192_MODE_CLKSRC(pdata->clock_source_sel) |
> + AD7192_MODE_CLKSRC(st->clock_sel) |
> AD7192_MODE_RATE(480);
>
> st->conf = AD7192_CONF_GAIN(0);
> @@ -499,7 +482,7 @@ static int ad7192_read_raw(struct iio_dev *indio_dev,
> *val -= 273 * ad7192_get_temp_scale(unipolar);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_SAMP_FREQ:
> - *val = st->mclk /
> + *val = st->fclk /
> (st->f_order * 1024 * AD7192_MODE_RATE(st->mode));
> return IIO_VAL_INT;
> }
> @@ -546,7 +529,7 @@ static int ad7192_write_raw(struct iio_dev *indio_dev,
> break;
> }
>
> - div = st->mclk / (val * st->f_order * 1024);
> + div = st->fclk / (val * st->f_order * 1024);
> if (div < 1 || div > 1023) {
> ret = -EINVAL;
> break;
> @@ -625,6 +608,42 @@ static const struct iio_chan_spec ad7193_channels[] = {
> IIO_CHAN_SOFT_TIMESTAMP(14),
> };
>
> +static int ad7192_clock_select(struct spi_device *spi, struct ad7192_state
> *st)
> +{
> + int ret;
> +
> + st->clock_sel = AD7192_CLK_EXT_MCLK2;
> + st->mclk = devm_clk_get(>dev, "clk");
> + if (IS_ERR(st->mclk)) {
> + if (PTR_ERR(st->mclk) != -ENOENT)
> + return PTR_ERR(st->mclk);
> +
> + /* try xtal option */
> + st->mclk = devm_clk_get(>dev, "xtal");
I'm not seeing any other driver using clock naming to distinguish
between different clock options.
Which does raise the question of how to do this?
I think it's probably going to be a single clock as both the external
clock and the crystal would be connected to the same pins.
> + st->clock_sel = AD7192_CLK_EXT_MCLK1_2;
> + if (IS_ERR(st->mclk)) {
> + if (PTR_ERR(st->mclk) != -ENOENT)
> + return PTR_ERR(st->mclk);
> +
> +
[PATCH v3 2/3] Staging: iio: adt7316: Move interrupt related code
There is a function adt7316_irq_setup() where irq_type is being
set. It would be good to move devm_request_threaded_irq() function
and assignment of chip->config1 in adt7316_irq_setup() to unclutter
the code in probe function.
Signed-off-by: Shreeya Patel
---
drivers/staging/iio/addac/adt7316.c | 34 ++---
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316.c
b/drivers/staging/iio/addac/adt7316.c
index 86b2c3d53588..97dd48153293 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -1807,11 +1807,12 @@ static irqreturn_t adt7316_event_handler(int irq, void
*private)
return IRQ_HANDLED;
}
-static int adt7316_setup_irq(struct device *dev, int irq)
+static int adt7316_setup_irq(struct iio_dev *indio_dev)
{
- int irq_type;
+ struct adt7316_chip_info *chip = iio_priv(indio_dev);
+ int irq_type, ret;
- irq_type = irqd_get_trigger_type(irq_get_irq_data(irq));
+ irq_type = irqd_get_trigger_type(irq_get_irq_data(chip->bus.irq));
switch (irq_type) {
case IRQF_TRIGGER_HIGH:
@@ -1821,13 +1822,23 @@ static int adt7316_setup_irq(struct device *dev, int
irq)
case IRQF_TRIGGER_FALLING:
break;
default:
- dev_info(dev, "mode %d unsupported, using IRQF_TRIGGER_LOW\n",
+ dev_info(_dev->dev, "mode %d unsupported, using
IRQF_TRIGGER_LOW\n",
irq_type);
irq_type = IRQF_TRIGGER_LOW;
break;
}
- return irq_type;
+ ret = devm_request_threaded_irq(_dev->dev, chip->bus.irq,
+ NULL, adt7316_event_handler,
+ irq_type | IRQF_ONESHOT,
+ indio_dev->name, indio_dev);
+ if (ret)
+ return ret;
+
+ if (irq_type & IRQF_TRIGGER_HIGH)
+ chip->config1 |= ADT7316_INT_POLARITY;
+
+ return ret;
}
/*
@@ -2124,7 +2135,6 @@ int adt7316_probe(struct device *dev, struct adt7316_bus
*bus,
{
struct adt7316_chip_info *chip;
struct iio_dev *indio_dev;
- int irq_type;
int ret = 0;
indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
@@ -2168,19 +2178,9 @@ int adt7316_probe(struct device *dev, struct adt7316_bus
*bus,
indio_dev->modes = INDIO_DIRECT_MODE;
if (chip->bus.irq > 0) {
- irq_type = adt7316_setup_irq(dev, chip->bus.irq);
-
- ret = devm_request_threaded_irq(dev, chip->bus.irq,
- NULL,
- adt7316_event_handler,
- irq_type | IRQF_ONESHOT,
- indio_dev->name,
- indio_dev);
+ ret = adt7316_setup_irq(indio_dev);
if (ret)
return ret;
-
- if (irq_type & IRQF_TRIGGER_HIGH)
- chip->config1 |= ADT7316_INT_POLARITY;
}
ret = chip->bus.write(chip->bus.client, ADT7316_CONFIG1, chip->config1);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/3] Remove platform data and introduce DT bindings
This patchset introduces the use of device tree bindings for setting up the irq_type and removes the usage of platform data. Also, code related to interrupt is moved to the new function adt7316_setup_irq to unclutter the code in adt7316_probe(). A dev_err() message is added to give more details about the error. Shreeya Patel (3): Staging: iio: adt7316: Use device tree data to assign irq_type Staging: iio: adt7316: Move interrupt related code Staging: iio: adt7316: Add a dev_err() message drivers/staging/iio/addac/adt7316.c | 52 + 1 file changed, 38 insertions(+), 14 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/3] Staging: iio: adt7316: Use device tree data to assign irq_type
ADT7316 driver no more uses platform data and hence use device tree
data instead of platform data for assigning irq_type field.
Switch case figures out the type of irq and if it's the default case
then assign the default value to the irq_type i.e.
irq_type = IRQF_TRIGGER_LOW
All this is implemented in a new function called adt7316_setup_irq.
Signed-off-by: Shreeya Patel
---
drivers/staging/iio/addac/adt7316.c | 29 +
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/addac/adt7316.c
b/drivers/staging/iio/addac/adt7316.c
index 9c72538baf9e..86b2c3d53588 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -1807,6 +1807,29 @@ static irqreturn_t adt7316_event_handler(int irq, void
*private)
return IRQ_HANDLED;
}
+static int adt7316_setup_irq(struct device *dev, int irq)
+{
+ int irq_type;
+
+ irq_type = irqd_get_trigger_type(irq_get_irq_data(irq));
+
+ switch (irq_type) {
+ case IRQF_TRIGGER_HIGH:
+ case IRQF_TRIGGER_RISING:
+ break;
+ case IRQF_TRIGGER_LOW:
+ case IRQF_TRIGGER_FALLING:
+ break;
+ default:
+ dev_info(dev, "mode %d unsupported, using IRQF_TRIGGER_LOW\n",
+irq_type);
+ irq_type = IRQF_TRIGGER_LOW;
+ break;
+ }
+
+ return irq_type;
+}
+
/*
* Show mask of enabled interrupts in Hex.
*/
@@ -2101,8 +2124,7 @@ int adt7316_probe(struct device *dev, struct adt7316_bus
*bus,
{
struct adt7316_chip_info *chip;
struct iio_dev *indio_dev;
- unsigned short *adt7316_platform_data = dev->platform_data;
- int irq_type = IRQF_TRIGGER_LOW;
+ int irq_type;
int ret = 0;
indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
@@ -2146,8 +2168,7 @@ int adt7316_probe(struct device *dev, struct adt7316_bus
*bus,
indio_dev->modes = INDIO_DIRECT_MODE;
if (chip->bus.irq > 0) {
- if (adt7316_platform_data[0])
- irq_type = adt7316_platform_data[0];
+ irq_type = adt7316_setup_irq(dev, chip->bus.irq);
ret = devm_request_threaded_irq(dev, chip->bus.irq,
NULL,
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/3] Staging: iio: adt7316: Add a dev_err() message
Add a dev_err() message "failed to request irq" for describing
what went wrong when an error contition is statisfied.
Signed-off-by: Shreeya Patel
---
drivers/staging/iio/addac/adt7316.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/addac/adt7316.c
b/drivers/staging/iio/addac/adt7316.c
index 97dd48153293..e3eb8ad06403 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -1832,8 +1832,11 @@ static int adt7316_setup_irq(struct iio_dev *indio_dev)
NULL, adt7316_event_handler,
irq_type | IRQF_ONESHOT,
indio_dev->name, indio_dev);
- if (ret)
+ if (ret) {
+ dev_err(_dev->dev, "failed to request irq %d\n",
+ chip->bus.irq);
return ret;
+ }
if (irq_type & IRQF_TRIGGER_HIGH)
chip->config1 |= ADT7316_INT_POLARITY;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Revert "Staging: iio: adt7316: Add an extra check for 'ret' equals to 0"
On Sat, 08 Dec 2018 00:07:21 +0530
Shreeya Patel wrote:
> On Thu, 2018-12-06 at 15:40 +0300, Dan Carpenter wrote:
> > On Wed, Dec 05, 2018 at 02:59:53PM -0700, Jeremy Fertic wrote:
> > > On Thu, Dec 06, 2018 at 01:25:55AM +0530, Shreeya Patel wrote:
> > > > On Tue, 2018-12-04 at 18:49 -0700, Jeremy Fertic wrote:
> > > > > This reverts commit 00426e99789357dbff7e719a092ce36a3ce49d94.
> > > > >
> > > > > i2c_smbus_read_byte() returns 0 when a byte with the value 0 is
> > > > > read
> > > > > from
> > > > > the device. This is a valid read so revert the check for 0.
> > > > >
> > > > > Signed-off-by: Jeremy Fertic
> > > > > ---
> > > >
> > > > Hi Jeremy,
> > > >
> > > > As per my understanding, 0 value indicates no error but no data
> > > > read.
> > > > Then how can this be a valid case?
> > > >
> > > > Can you please make me understand that how can we consider this
> > > > as a
> > > > valid case even when no data has been read?
> >
> > It's not reading no data. It's reading one byte of data and
> > returning
> > it.
> >
> > > >
> > > >
> > > > Thanks
> > >
> > > I'm not sure I understand why the value 0 would indicate no data
> > > read.
> > > Doesn't that just mean a byte was read with the value 0.
> >
> > Yes. It does mean that. Please don't ask rhetorical
> > questions... :(
> > This list is full of people who can't resist answering every
> > question.
> >
> > > For instance, if the input to the adc is 0V. Can you point me to
> > > where
> > > you're seeing that this would indicate no data read?
> >
> > drivers/i2c/i2c-core-smbus.c
> > 88 /**
> > 89 * i2c_smbus_read_byte - SMBus "receive byte" protocol
> > 90 * @client: Handle to slave device
> > 91 *
> > 92 * This executes the SMBus "receive byte" protocol, returning
> > negative errno
> > 93 * else the byte received from the device.
> > 94 */
> > 95 s32 i2c_smbus_read_byte(const struct i2c_client *client)
> > 96 {
> > 97 union i2c_smbus_data data;
> > 98 int status;
> > 99
> >100 status = i2c_smbus_xfer(client->adapter, client-
> > >addr, client->flags,
> >101 I2C_SMBUS_READ, 0,
> >102 I2C_SMBUS_BYTE, );
> >103 return (status < 0) ? status : data.byte;
> > ^
> >104 }
> >105 EXPORT_SYMBOL(i2c_smbus_read_byte);
>
> Even I had sent the same code to Jonathan and we had a discussion on
> this.
> I asked him that this code clearly shows that there is an error
> condition only when status < 0 then why do we need a check for status =
> 0.
>
> Then he explained me that 0 isn't an error. The issue is the silliness
> of the i2c interface.
>
> Pretty much every other bus returns an error (negative) if less data is
> received than expected. Most i2c
> bus master's do as well but in theory it can return 0 to indicate no
> error but no data read (which doesn't make any sense)
>
> 0 doesn't ever happen in reality but it should be handled for
> correctness though.
>
> So we should wait for what Jonathan has to say on this :)
Yup, I was being an idiot. Sorry about that! For some reason I'd
gotten it into my head that the particular function we were talking
about was i2c_master_send which does indeed do as discussed above.
Apologies for misleading you on this. Definitely a proper idiot
moment of me not reading what the code actually was properly, even
when you questioned what I was going on about.
Thanks to Jeremy for catching this one.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Jonathan
>
> Thanks
>
> > You are right. Commit 00426e997893 ("Staging: iio: adt7316: Add an
> > extra check for 'ret' equals to 0") needs to be reverted...
> >
> > regards,
> > dan carpenter
> >
> >
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad5933: add binding doc for ad5933
On Sun, 02 Dec 2018 15:22:15 -0800 Joe Perches wrote: > On Mon, 2018-12-03 at 00:20 +0530, Himanshu Jha wrote: > > On Sun, Dec 02, 2018 at 02:57:12PM -0200, Marcelo Schmitt wrote: > > > Add a devicetree documentation for the ad5933 and ad5934 impedance > > > converter, network analyzer. > > > > > > Co-Developed-by: Gabriel Capella > > > > checkpatch spits out: > > > > WARNING: Non-standard signature: Co-Developed-by: > > > > Co-developed-by Vs Co-Developed-by ? > > > > Documentation/process/5.Posting.rst: - Co-developed-by: states that the > > patch was also created by another developer > > Documentation/process/submitting-patches.rst:12) When to use Acked-by:, > > Cc:, and Co-Developed-by: > > > > Confusing! Don't know which one is correct. > > I think neither one. > > What's the real purpose or value of it? > There isn't one as far as I can tell. > > Just use Signed-off-by: > > Or maybe add multiple "Authored-by:" if > anyone is all that concerned about authorship > crediting... This is output of pair programming so only fair to acknowledge both developers (or more if a larger group). Right now we have a guide that says Co-developed-by is the way to do that. I would stick to that. If people feel something else makes sense then they should propose a change to the documentation and hopefully we can reach some agreement on this. I'm happy with Co-developed-by in IIO as I think it's a fair reflection of what happened. Authored-by would be fine but isn't a standard tag documented anywhere. Jonathan > > > > > > > Signed-off-by: Marcelo Schmitt > > > Signed-off-by: Gabriel Capella > > > --- > > > > Use `./scripts/get_maintainer.pl ` to list the DT > > maintainers and the relevant mailing list. > > > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH -next] media: rockchip/vpu: remove set but not used variables 'luma_qtable_p, chroma_qtable_p'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c:101:10: warning:
variable 'chroma_qtable_p' set but not used [-Wunused-but-set-variable]
drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c:100:10: warning:
variable 'luma_qtable_p' set but not used [-Wunused-but-set-variable]
drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c:70:10: warning:
variable 'chroma_qtable_p' set but not used [-Wunused-but-set-variable]
drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c:69:10: warning:
variable 'luma_qtable_p' set but not used [-Wunused-but-set-variable]
It never used since introduction in
commit 775fec69008d ("media: add Rockchip VPU JPEG encoder driver")
Signed-off-by: YueHaibing
---
drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c | 5 -
drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c | 5 -
2 files changed, 10 deletions(-)
diff --git a/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c
b/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c
index e27c108..5282236 100644
--- a/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c
+++ b/drivers/staging/media/rockchip/vpu/rk3288_vpu_hw_jpeg_enc.c
@@ -66,13 +66,8 @@ rk3288_vpu_jpeg_enc_set_qtable(struct rockchip_vpu_dev *vpu,
unsigned char *luma_qtable,
unsigned char *chroma_qtable)
{
- __be32 *luma_qtable_p;
- __be32 *chroma_qtable_p;
u32 reg, i;
- luma_qtable_p = (__be32 *)luma_qtable;
- chroma_qtable_p = (__be32 *)chroma_qtable;
-
for (i = 0; i < VEPU_JPEG_QUANT_TABLE_COUNT; i++) {
reg = get_unaligned_be32(_qtable[i]);
vepu_write_relaxed(vpu, reg, VEPU_REG_JPEG_LUMA_QUAT(i));
diff --git a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
index 5f75e4d..dbc86d9 100644
--- a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
+++ b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
@@ -97,13 +97,8 @@ rk3399_vpu_jpeg_enc_set_qtable(struct rockchip_vpu_dev *vpu,
unsigned char *luma_qtable,
unsigned char *chroma_qtable)
{
- __be32 *luma_qtable_p;
- __be32 *chroma_qtable_p;
u32 reg, i;
- luma_qtable_p = (__be32 *)luma_qtable;
- chroma_qtable_p = (__be32 *)chroma_qtable;
-
for (i = 0; i < VEPU_JPEG_QUANT_TABLE_COUNT; i++) {
reg = get_unaligned_be32(_qtable[i]);
vepu_write_relaxed(vpu, reg, VEPU_REG_JPEG_LUMA_QUAT(i));
--
2.7.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
