* Re: [PATCH v4 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Serge Semin @ 2017-01-13 9:47 UTC (permalink / raw)
To: Greg KH
Cc: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, andrew-g2DYL2Zd6BY,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
Sergey.Semin-vHJ8rsvMqnUPfZBKTuL5GA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170113072235.GA12825-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Fri, Jan 13, 2017 at 08:22:35AM +0100, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Fri, Jan 13, 2017 at 01:54:17AM +0300, Serge Semin wrote:
> > On Wed, Jan 11, 2017 at 09:21:19AM +0100, Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> > > > + /* Return failure if root directory doesn't exist */
> > > > + if (!csr_dbgdir) {
> > > > + dev_dbg(dev, "No Debugfs root directory");
> > > > + return -EINVAL;
> > > > + }
> > >
> > > If debugfs is not enabled, don't error out, just keep going, it should
> > > never stop kernel code from running properly.
> > >
> > > Also, this test isn't really doing what you think it is doing...
> > >
> >
> > I see, it must be replaced with IS_ERR_OR_NULL() test.
>
> No! That's a pain, when the debugfs interface was created its goal was
> to make it _easy_ to use, not hard. IS_ERR_OR_NULL() is hard, and
> messy, don't do that.
>
> > But I don't think,
> > it would be good to get rid of dev_dbg() completely here. In case if
> > debugging is enabled, user would understand why csr-node isn't created within
> > DebugFS directory. I don't see the reasoning why one shouldn't know a source
> > of possible problems.
> > (See the next comment as continue of the discussion)
>
> Why would a user care about debugfs?
>
> > > > + /* Create Debugfs directory for CSR file */
> > > > + snprintf(fname, CSRNAME_LEN, "%d-%04hx", cli->adapter->nr, cli->addr);
> > > > + pdev->csr_dir = debugfs_create_dir(fname, csr_dbgdir);
> > > > + if (IS_ERR_OR_NULL(pdev->csr_dir)) {
> > > > + dev_err(dev, "Failed to create CSR node directory");
> > > > + return -EINVAL;
> > >
> > > Again, don't do this, you really don't care if debugfs worked or not.
> > >
> >
> > Actually the driver doesn't stop the kernel code from running, if it finds out
> > any problem with DebugFS CSR-node creation. The function just logs the error
> > and return error status. Take a look the place the method is called:
> > 1489 /* Create debugfs files */
> > 1490 (void)idt_create_dbgfs_files(pdev);
> > The initialization code doesn't check the return value at all, so the driver
> > will proceed with further code.
> > Why did I make the function with return value? Because it's a good style to
> > always return a status of function code execution if it may fail, but only
> > caller will decide whether to check the return value or not.
>
> There is only one type of error that a debugfs call can return, and that
> is if debugfs is not enabled in the build. That's it, you don't need to
> care about any of that.
>
> > Regarding the error printing. In case if the code gets to this check, one can
> > be sure the DebugFS works properly, so in case if the driver failed to create
> > the corresponding sub-directory or node, it is really error to have any failure
> > at this point, and a user should be notified. But still the driver won't stop
> > functioning, since the caller doesn't check the return value.
> >
> > Hopefully you'll understand my point.
>
> Please understand mine, debugfs is supposed to be easy to use, you are
> not testing things properly here, and when you are, it doesn't matter.
> Just call the functions, save the return results if you need to (for
> dentries and the like), and move on. No error handling needed AT ALL!
>
> Yes, it feels "odd" for kernel code, but remember, this is only for
> debugging. Your code should not have any different codepaths for if the
> debugging logic worked or not. It doesn't care at all. So please, make
> it simple.
>
> > > > + dev_dbg(dev, "Debugfs-files created");
> > >
> > > You do know about ftrace, right? Please remove all of these
> > > "trace-like" debugging lines, they aren't needed for anyone.
> > >
> >
> > Ok, I'll remove all these prints, even though I do find these prints being
> > handy to have initialization process printed on debugging stage.
>
> Then use ftrace, that is what it is there for, don't roll your own
> driver-specific-functionality please.
>
> thanks,
>
> greg k-h
Ok, I see your point and do as you say.
Thanks,
Serge
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v4 0/2] iio: adc: Add Maxim MAX11100 driver
From: Jacopo Mondi @ 2017-01-13 9:50 UTC (permalink / raw)
To: wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
geert-Td1EMuHUCqxL1ZNQvxDV9g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
Hello,
sending out v4 splitting device tree bindings documentation and actual ADC
driver.
No changes in driver code since v3.
Same question for iio maintainers here:
I would like to have clarified the measure unit returned by read_raw().
Currently (value_raw * value_scale) return the ADC input value in mV.
While testing the patch I've been questioned if that should not actually
be in uV. This is easily achievable making _scale return a value in uV.
I have found no mention of this in the ABI documentation as it speaks of
generic voltage.
Can we have a final word on this?
Thanks Marek for having tested this.
v1 -> v2:
- incorporated pmeerw's review comments
- retrieve vref from dts and use that to convert read_raw result
to mV
- add device tree bindings documentation
v2 -> v3:
- add _SCALE bit of read_raw function and change _RAW bit accordingly
- call regulator_get_voltage when accessing the _SCALE part of read_raw
and not during probe
- add back remove function as regulator has to be disabled when detaching
the module. Do not use devm_ version of iio_register/unregister functions
anymore but do unregister in the remove.
- remove mutex as access to SPI bus is protected by SPI core. Thanks marex
v3 -> v4:
- split device tree binding documentation and actual ADC driver
- add "reg" to the list of required properties and use a better
namimg for the adc device node in bindings documentation as suggested
by Geert.
Jacopo Mondi (2):
iio: adc: Add Maxim MAX11100 driver
dt-bindings: iio: document MAX11100 ADC
.../devicetree/bindings/iio/adc/max11100.txt | 19 +++
drivers/iio/adc/Kconfig | 9 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++
4 files changed, 216 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
create mode 100644 drivers/iio/adc/max11100.c
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v4 1/2] iio: adc: Add Maxim MAX11100 driver
From: Jacopo Mondi @ 2017-01-13 9:50 UTC (permalink / raw)
To: wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/,
magnus.damm-Re5JQEeQqe8AvxtiuMwx3w, jic23-DgEjT+Ai2ygdnm+yROfE0A,
knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
pmeerw-jW+XmwGofnusTnJN9+BGXg, marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
geert-Td1EMuHUCqxL1ZNQvxDV9g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484301038-16386-1-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
From: Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Add iio driver for Maxim MAX11100 single-channel ADC.
Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Tested-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
drivers/iio/adc/Kconfig | 9 +++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/max11100.c | 187 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 197 insertions(+)
create mode 100644 drivers/iio/adc/max11100.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 38bc319..c32bc7a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -307,6 +307,15 @@ config MAX1027
To compile this driver as a module, choose M here: the module will be
called max1027.
+config MAX11100
+ tristate "Maxim max11100 ADC driver"
+ depends on SPI_MASTER
+ help
+ Say yes here to build support for Maxim max11100 SPI ADC
+
+ To compile this driver as a module, choose M here: the module will be
+ called max11100.
+
config MAX1363
tristate "Maxim max1363 ADC driver"
depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d36c4be..5684369 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
obj-$(CONFIG_LPC18XX_ADC) += lpc18xx_adc.o
obj-$(CONFIG_LTC2485) += ltc2485.o
obj-$(CONFIG_MAX1027) += max1027.o
+obj-$(CONFIG_MAX11100) += max11100.o
obj-$(CONFIG_MAX1363) += max1363.o
obj-$(CONFIG_MCP320X) += mcp320x.o
obj-$(CONFIG_MCP3422) += mcp3422.o
diff --git a/drivers/iio/adc/max11100.c b/drivers/iio/adc/max11100.c
new file mode 100644
index 0000000..78e2a45
--- /dev/null
+++ b/drivers/iio/adc/max11100.c
@@ -0,0 +1,187 @@
+/*
+ * iio/adc/max11100.c
+ * Maxim max11100 ADC Driver with IIO interface
+ *
+ * Copyright (C) 2016 Renesas Electronics Corporation
+ * Copyright (C) 2016 Jacopo Mondi
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/driver.h>
+
+/*
+ * LSB is the ADC single digital step
+ * 1 LSB = (vref_mv / 2 ^ 16)
+ *
+ * LSB is used to calculate analog voltage value
+ * from the number of ADC steps count
+ *
+ * Ain = (count * LSB)
+ */
+#define MAX11100_LSB_DIV (1 << 16)
+
+struct max11100_state {
+ const struct max11100_chip_desc *desc;
+ struct regulator *vref_reg;
+ struct spi_device *spi;
+};
+
+static struct iio_chan_spec max11100_channels[] = {
+ { /* [0] */
+ .type = IIO_VOLTAGE,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+ BIT(IIO_CHAN_INFO_SCALE),
+ },
+};
+
+static struct max11100_chip_desc {
+ unsigned int num_chan;
+ const struct iio_chan_spec *channels;
+} max11100_desc = {
+ .num_chan = ARRAY_SIZE(max11100_channels),
+ .channels = max11100_channels,
+};
+
+static int max11100_read_single(struct iio_dev *indio_dev, int *val)
+{
+ int ret;
+ struct max11100_state *state = iio_priv(indio_dev);
+ uint8_t buffer[3];
+
+ ret = spi_read(state->spi, buffer, sizeof(buffer));
+ if (ret) {
+ dev_err(&indio_dev->dev, "SPI transfer failed\n");
+ return ret;
+ }
+
+ /* the first 8 bits sent out from ADC must be 0s */
+ if (buffer[0]) {
+ dev_err(&indio_dev->dev, "Invalid value: buffer[0] != 0\n");
+ return -EINVAL;
+ }
+
+ *val = (buffer[1] << 8) | buffer[2];
+
+ return 0;
+}
+
+static int max11100_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long info)
+{
+ int ret, vref_uv;
+ struct max11100_state *state = iio_priv(indio_dev);
+
+ switch (info) {
+ case IIO_CHAN_INFO_RAW:
+ ret = max11100_read_single(indio_dev, val);
+ if (ret)
+ return ret;
+
+ return IIO_VAL_INT;
+
+ case IIO_CHAN_INFO_SCALE:
+ vref_uv = regulator_get_voltage(state->vref_reg);
+ if (vref_uv < 0)
+ /* dummy regulator "get_voltage" returns -EINVAL */
+ return -EINVAL;
+
+ *val = vref_uv / 1000;
+ *val2 = MAX11100_LSB_DIV;
+ return IIO_VAL_FRACTIONAL;
+ }
+
+ return -EINVAL;
+}
+
+static const struct iio_info max11100_info = {
+ .driver_module = THIS_MODULE,
+ .read_raw = max11100_read_raw,
+};
+
+static int max11100_probe(struct spi_device *spi)
+{
+ int ret;
+ struct iio_dev *indio_dev;
+ struct max11100_state *state;
+
+ indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*state));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ spi_set_drvdata(spi, indio_dev);
+
+ state = iio_priv(indio_dev);
+ state->spi = spi;
+ state->desc = &max11100_desc;
+
+ indio_dev->dev.parent = &spi->dev;
+ indio_dev->dev.of_node = spi->dev.of_node;
+ indio_dev->info = &max11100_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = state->desc->channels;
+ indio_dev->num_channels = state->desc->num_chan;
+
+ state->vref_reg = devm_regulator_get(&spi->dev, "vref");
+ if (IS_ERR(state->vref_reg))
+ return PTR_ERR(state->vref_reg);
+
+ ret = regulator_enable(state->vref_reg);
+ if (ret)
+ return ret;
+
+ ret = iio_device_register(indio_dev);
+ if (ret)
+ goto disable_regulator;
+
+ return 0;
+
+disable_regulator:
+ regulator_disable(state->vref_reg);
+
+ return ret;
+}
+
+static int max11100_remove(struct spi_device *spi)
+{
+ struct iio_dev *indio_dev = spi_get_drvdata(spi);
+ struct max11100_state *state = iio_priv(indio_dev);
+
+ regulator_disable(state->vref_reg);
+
+ iio_device_unregister(indio_dev);
+
+ return 0;
+}
+
+static const struct of_device_id max11100_ids[] = {
+ {.compatible = "maxim,max11100"},
+ { },
+};
+MODULE_DEVICE_TABLE(of, max11100_ids);
+
+static struct spi_driver max11100_driver = {
+ .driver = {
+ .name = "max11100",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(max11100_ids),
+ },
+ .probe = max11100_probe,
+ .remove = max11100_remove,
+};
+
+module_spi_driver(max11100_driver);
+
+MODULE_AUTHOR("Jacopo Mondi <jacopo-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>");
+MODULE_DESCRIPTION("Maxim max11100 ADC Driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
^ permalink raw reply related
* [PATCH v4 2/2] dt-bindings: iio: document MAX11100 ADC
From: Jacopo Mondi @ 2017-01-13 9:50 UTC (permalink / raw)
To: wsa+renesas, magnus.damm, jic23, knaack.h, lars, pmeerw,
marek.vasut, geert, robh+dt, mark.rutland
Cc: linux-iio, devicetree, linux-renesas-soc
In-Reply-To: <1484301038-16386-1-git-send-email-jacopo+renesas@jmondi.org>
Add device tree bindings documentation for Maxim MAX11100 single-channel
ADC
Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
.../devicetree/bindings/iio/adc/max11100.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/max11100.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/max11100.txt b/Documentation/devicetree/bindings/iio/adc/max11100.txt
new file mode 100644
index 0000000..ad0bc31
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/max11100.txt
@@ -0,0 +1,19 @@
+* Maxim max11100 Analog to Digital Converter (ADC)
+
+Required properties:
+ - compatible: Should be "maxim,max11100"
+ - reg: the adc unit address
+ - vref-supply: phandle to the regulator that provides reference voltage
+
+Optional properties:
+ - spi-max-frequency: SPI maximum frequency
+
+Example:
+
+max11100: adc@0 {
+ compatible = "maxim,max11100";
+ reg = <0>;
+ vref-supply = <&adc0_vref>;
+ spi-max-frequency = <240000>;
+};
+
--
2.7.4
^ permalink raw reply related
* Re: [PATCHv4 3/5] pinctrl: mvebu: pinctrl driver for 98DX3236 SoC
From: Sebastian Hesselbarth @ 2017-01-13 9:54 UTC (permalink / raw)
To: Chris Packham, linux-arm-kernel
Cc: Mark Rutland, Thomas Petazzoni, linux-gpio, Linus Walleij,
linux-kernel, Kalyan Kinthada, devicetree, Rob Herring,
Laxman Dewangan
In-Reply-To: <20170113091222.7132-4-chris.packham@alliedtelesis.co.nz>
On 13.01.2017 10:12, Chris Packham wrote:
> From: Kalyan Kinthada <kalyan.kinthada@alliedtelesis.co.nz>
>
> This pinctrl driver supports the 98DX3236, 98DX3336 and 98DX4251 SoCs
> from Marvell.
>
> Signed-off-by: Kalyan Kinthada <kalyan.kinthada@alliedtelesis.co.nz>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> ---
>
> Notes:
> Changes in v2:
> - include sdio support for the 98DX4251
> Changes in v3:
> - None
> Changes in v4:
> - Correct some discrepencies between binding and driver.
Well, unfortunately I still see differences between the "gpio" in
the binding and "gpo" in the driver.
Please go back to that list I sent you yesterday and fix them all.
[...]
> diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,armada-98dx3236-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/marvell,armada-98dx3236-pinctrl.txt
> new file mode 100644
> index 000000000000..b5bd23992fdf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/marvell,armada-98dx3236-pinctrl.txt
> @@ -0,0 +1,46 @@
[...]
> +mpp6 6 gpio, sd0(clk), dev(a2)
e.g. this is "gpio" ...
[...]
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-xp.c b/drivers/pinctrl/mvebu/pinctrl-armada-xp.c
> index e4ea71a9d985..9601d662c7f5 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-xp.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-xp.c
> @@ -49,6 +49,10 @@ enum armada_xp_variant {
[...]
> + MPP_MODE(6,
> + MPP_VAR_FUNCTION(0x0, "gpo", NULL, V_98DX3236_PLUS),
... but here it is "gpo".
Sebastian
^ permalink raw reply
* Re: [PATCH] ARM64: dts: meson-gxbb-odroidc2: fix GbE tx link breakage
From: Jerome Brunet @ 2017-01-13 9:55 UTC (permalink / raw)
To: Kevin Hilman
Cc: Carlo Caione, linux-amlogic, devicetree, linux-arm-kernel,
linux-kernel
In-Reply-To: <m2r3476atl.fsf@baylibre.com>
On Thu, 2017-01-12 at 16:52 -0800, Kevin Hilman wrote:
> Jerome Brunet <jbrunet@baylibre.com> writes:
>
> >
> > OdroidC2 GbE link breaks under heavy tx transfer. This happens even
> > if the
> > MAC does not enable Energy Efficient Ethernet (No Low Power state
> > Idle on
> > the Tx path). The problem seems to come from the phy Rx path,
> > entering the
> > LPI state.
> >
> > Disabling EEE advertisement on the phy prevent this feature to be
> > negociated with the link partner and solve the issue.
> >
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >
> > This patch is based on Linus recent master branch [0]
> > This patch depends on the series [1] which has been merged in this
> > branch.
> >
> > 0: ba6d973f78eb ("Merge
> > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net")
> > 1: http://lkml.kernel.org/r/1480326409-25419-1-git-send-email-jbrun
> > et@baylibre.com
> > Fix integration of eee-broken-modes
> >
> > arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 12
> > ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> > b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> > index 238fbeacd330..d8933e9e9a5a 100644
> > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
> > @@ -147,6 +147,18 @@
> > status = "okay";
> > pinctrl-0 = <ð_rgmii_pins>;
> > pinctrl-names = "default";
> > + phy-handle = <ð_phy0>;
> > +
> > + mdio {
> > + compatible = "snps,dwmac-mdio";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + eth_phy0: ethernet-phy@0 {
> > + reg = <0>;
> > + eee-broken-1000t;
> > + };
> > + };
>
> There's already an MDIO node in the meson-gx.dtsi (using the same
> compatible), shouldn't you just override that and add the new
> properties?
Hum, yeah in the changes queued from 4.11, right ?
But not in the latest rc from Linus. Since this is a fix, it was
actually target for this branch.
I guess you'd prefer to avoid merge conflicts when the 4.10 is tagged.
How do you want to handle this ?
>
> What would make things easier is if the names were like Martin used
> in
> his reset patch, so that when I merge them together it's not a major
> conflict.
Sure, no problem. I'll change it to match what has been done by Martin.
>
> Thanks,
>
> Kevin
>
> [1] https://patchwork.kernel.org/patch/9459409/
^ permalink raw reply
* Re: [PATCH v4 2/2] dt-bindings: iio: document MAX11100 ADC
From: Geert Uytterhoeven @ 2017-01-13 10:04 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Wolfram Sang, Magnus Damm, Jonathan Cameron, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald, Marek Vasut, Rob Herring,
Mark Rutland, linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Renesas
In-Reply-To: <1484301038-16386-3-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
On Fri, Jan 13, 2017 at 10:50 AM, Jacopo Mondi
<jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org> wrote:
> Add device tree bindings documentation for Maxim MAX11100 single-channel
> ADC
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 1/2] mmc: mediatek: Use data tune for CMD line tune
From: Yong Mao @ 2017-01-13 10:17 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Mark Rutland, Matthias Brugger, Catalin Marinas,
Will Deacon, Chaotian Jing, Philipp Zabel, Greg Kroah-Hartman,
Eddie Huang, Chunfeng Yun, Nicolas Boichat, Douglas Anderson,
Javier Martinez Canillas, linux-mmc@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-mediatek
In-Reply-To: <CAPDyKFq0_WpCAAmUDfaKApq7=tdpjNqL896FC=QPfGgE9JaEHA@mail.gmail.com>
On Thu, 2017-01-12 at 11:39 +0100, Ulf Hansson wrote:
> On 12 January 2017 at 11:04, Yong Mao <yong.mao@mediatek.com> wrote:
> > From: yong mao <yong.mao@mediatek.com>
> >
> > CMD response CRC error may cause cannot boot up
> > Change to use data tune for CMD line
> > Separate cmd internal delay for HS200/HS400 mode
>
> Please try to work a little bit on improving the change log. Moreover
> as this is a fix for a regression (it seems like so?), please try to
> make that clear.
This change can fix CMD respose CRC issue in our platform.
I will try to make it clear in next version.
>
> >
> > Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
> > ---
> > arch/arm64/boot/dts/mediatek/mt8173-evb.dts | 3 +
>
> Changes to the DTS files should be a separate change. Please split it
> into its own patch.
>
> > drivers/mmc/host/mtk-sd.c | 169 +++++++++++++++++++++++----
> > 2 files changed, 149 insertions(+), 23 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> > index 0ecaad4..29c3100 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
> > @@ -134,6 +134,9 @@
> > bus-width = <8>;
> > max-frequency = <50000000>;
> > cap-mmc-highspeed;
> > + hs200-cmd-int-delay = <26>;
> > + hs400-cmd-int-delay = <14>;
> > + cmd-resp-sel = <0>; /* 0: rising, 1: falling */
> > vmmc-supply = <&mt6397_vemc_3v3_reg>;
> > vqmmc-supply = <&mt6397_vio18_reg>;
> > non-removable;
> > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> > index 80ba034..93eb395 100644
> > --- a/drivers/mmc/host/mtk-sd.c
> > +++ b/drivers/mmc/host/mtk-sd.c
> > @@ -75,6 +75,7 @@
> > #define MSDC_PATCH_BIT1 0xb4
> > #define MSDC_PAD_TUNE 0xec
> > #define PAD_DS_TUNE 0x188
> > +#define PAD_CMD_TUNE 0x18c
> > #define EMMC50_CFG0 0x208
> >
> > /*--------------------------------------------------------------------------*/
> > @@ -210,12 +211,17 @@
> > #define MSDC_PATCH_BIT_SPCPUSH (0x1 << 29) /* RW */
> > #define MSDC_PATCH_BIT_DECRCTMO (0x1 << 30) /* RW */
> >
> > -#define MSDC_PAD_TUNE_DATRRDLY (0x1f << 8) /* RW */
> > -#define MSDC_PAD_TUNE_CMDRDLY (0x1f << 16) /* RW */
> > +#define MSDC_PAD_TUNE_DATWRDLY (0x1f << 0) /* RW */
> > +#define MSDC_PAD_TUNE_DATRRDLY (0x1f << 8) /* RW */
> > +#define MSDC_PAD_TUNE_CMDRDLY (0x1f << 16) /* RW */
> > +#define MSDC_PAD_TUNE_CMDRRDLY (0x1f << 22) /* RW */
>
> Is there a white space change somewhere here? I don't see any changes
> to MSDC_PAD_TUNE_DATRRDLY and MSDC_PAD_TUNE_CMDRDLY.
>
Sorry. I will fix in next version.
> > +#define MSDC_PAD_TUNE_CLKTDLY (0x1f << 27) /* RW */
> >
> > -#define PAD_DS_TUNE_DLY1 (0x1f << 2) /* RW */
> > -#define PAD_DS_TUNE_DLY2 (0x1f << 7) /* RW */
> > -#define PAD_DS_TUNE_DLY3 (0x1f << 12) /* RW */
> > +#define PAD_DS_TUNE_DLY1 (0x1f << 2) /* RW */
> > +#define PAD_DS_TUNE_DLY2 (0x1f << 7) /* RW */
> > +#define PAD_DS_TUNE_DLY3 (0x1f << 12) /* RW */
> > +
>
> Ditto.
Ditto.
>
> > +#define PAD_CMD_TUNE_RX_DLY3 (0x1f << 1) /* RW */
> >
> > #define EMMC50_CFG_PADCMD_LATCHCK (0x1 << 0) /* RW */
> > #define EMMC50_CFG_CRCSTS_EDGE (0x1 << 3) /* RW */
> > @@ -236,7 +242,9 @@
> > #define CMD_TIMEOUT (HZ/10 * 5) /* 100ms x5 */
> > #define DAT_TIMEOUT (HZ * 5) /* 1000ms x5 */
> >
> > -#define PAD_DELAY_MAX 32 /* PAD delay cells */
> > +#define PAD_DELAY_MAX 32 /* PAD delay cells */
>
> Ditto.
>
Ditto.
> > +#define ENOUGH_MARGIN_MIN 12 /* Enough Margin */
> > +#define PREFER_START_POS_MAX 4 /* Prefer start position */
> > /*--------------------------------------------------------------------------*/
> > /* Descriptor Structure */
> > /*--------------------------------------------------------------------------*/
> > @@ -284,12 +292,14 @@ struct msdc_save_para {
> > u32 patch_bit0;
> > u32 patch_bit1;
> > u32 pad_ds_tune;
> > + u32 pad_cmd_tune;
> > u32 emmc50_cfg0;
> > };
> >
> > struct msdc_tune_para {
> > u32 iocon;
> > u32 pad_tune;
> > + u32 pad_cmd_tune;
> > };
> >
> > struct msdc_delay_phase {
> > @@ -331,6 +341,9 @@ struct msdc_host {
> > unsigned char timing;
> > bool vqmmc_enabled;
> > u32 hs400_ds_delay;
> > + u32 hs200_cmd_int_delay; /* cmd internal delay for HS200/SDR104 */
> > + u32 hs400_cmd_int_delay; /* cmd internal delay for HS400 */
> > + u32 hs200_cmd_resp_sel; /* cmd response sample selection */
> > bool hs400_mode; /* current eMMC will run at hs400 mode */
> > struct msdc_save_para save_para; /* used when gate HCLK */
> > struct msdc_tune_para def_tune_para; /* default tune setting */
> > @@ -596,12 +609,21 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
> > */
> > if (host->sclk <= 52000000) {
> > writel(host->def_tune_para.iocon, host->base + MSDC_IOCON);
> > - writel(host->def_tune_para.pad_tune, host->base + MSDC_PAD_TUNE);
> > + writel(host->def_tune_para.pad_tune,
> > + host->base + MSDC_PAD_TUNE);
>
> Please don't change code just because you feel like doing it. This is
> a completely unessarry change and it makes it harder for me to review.
>
> Can you go thorugh the complete patch and make sure to undo all
> similar changes, there are more of them.
>
Sorry. I will fix in next version.
> > } else {
> > - writel(host->saved_tune_para.iocon, host->base + MSDC_IOCON);
> > - writel(host->saved_tune_para.pad_tune, host->base + MSDC_PAD_TUNE);
> > + writel(host->saved_tune_para.iocon,
> > + host->base + MSDC_IOCON);
> > + writel(host->saved_tune_para.pad_tune,
> > + host->base + MSDC_PAD_TUNE);
> > + writel(host->saved_tune_para.pad_cmd_tune,
> > + host->base + PAD_CMD_TUNE);
> > }
> >
> > + if (timing == MMC_TIMING_MMC_HS400)
> > + sdr_set_field(host->base + PAD_CMD_TUNE,
> > + MSDC_PAD_TUNE_CMDRRDLY,
> > + host->hs400_cmd_int_delay);
> > dev_dbg(host->dev, "sclk: %d, timing: %d\n", host->sclk, timing);
> > }
> >
> > @@ -1302,7 +1324,8 @@ static struct msdc_delay_phase get_best_delay(struct msdc_host *host, u32 delay)
> > len_final = len;
> > }
> > start += len ? len : 1;
> > - if (len >= 8 && start_final < 4)
> > + if (len >= ENOUGH_MARGIN_MIN &&
> > + start_final < PREFER_START_POS_MAX)
> > break;
> > }
> >
> > @@ -1325,48 +1348,128 @@ static int msdc_tune_response(struct mmc_host *mmc, u32 opcode)
> > struct msdc_host *host = mmc_priv(mmc);
> > u32 rise_delay = 0, fall_delay = 0;
> > struct msdc_delay_phase final_rise_delay, final_fall_delay = { 0,};
> > + struct msdc_delay_phase internal_delay_phase;
> > u8 final_delay, final_maxlen;
> > + u32 internal_delay = 0;
> > int cmd_err;
> > - int i;
> > + int i, j;
> >
> > + if (mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
> > + mmc->ios.timing == MMC_TIMING_UHS_SDR104)
> > + sdr_set_field(host->base + MSDC_PAD_TUNE,
> > + MSDC_PAD_TUNE_CMDRRDLY,
> > + host->hs200_cmd_int_delay);
> > sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> > for (i = 0 ; i < PAD_DELAY_MAX; i++) {
> > sdr_set_field(host->base + MSDC_PAD_TUNE,
> > MSDC_PAD_TUNE_CMDRDLY, i);
> > - mmc_send_tuning(mmc, opcode, &cmd_err);
> > - if (!cmd_err)
> > - rise_delay |= (1 << i);
> > + for (j = 0; j < 3; j++) {
>
> Any reason to why looping three times makes sense? Maybe add a comment?
>
Using the same parameters, it may sometimes pass the test,
but sometimes it may fail. To make sure the parameters is
more stable, we test 3 times for on set of parameters.
Anyway, I will add comment here in next version.
> > + mmc_send_tuning(mmc, opcode, &cmd_err);
> > + if (!cmd_err) {
> > + rise_delay |= (1 << i);
> > + } else {
> > + rise_delay &= ~(1 << i);
> > + break;
> > + }
> > + }
> > }
> > final_rise_delay = get_best_delay(host, rise_delay);
> > /* if rising edge has enough margin, then do not scan falling edge */
> > - if (final_rise_delay.maxlen >= 10 ||
> > - (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
> > + if (final_rise_delay.maxlen >= ENOUGH_MARGIN_MIN &&
> > + final_rise_delay.start < PREFER_START_POS_MAX)
>
> This looks like clean-ups, as you are converting from magic numbers to
> defines. Please make this kind of changes separately.
I will drop this change in next version.
>
> > goto skip_fall;
> >
> > sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> > for (i = 0; i < PAD_DELAY_MAX; i++) {
> > sdr_set_field(host->base + MSDC_PAD_TUNE,
> > MSDC_PAD_TUNE_CMDRDLY, i);
> > - mmc_send_tuning(mmc, opcode, &cmd_err);
> > - if (!cmd_err)
> > - fall_delay |= (1 << i);
> > + for (j = 0; j < 3; j++) {
>
> 3?
>
> > + mmc_send_tuning(mmc, opcode, &cmd_err);
> > + if (!cmd_err) {
> > + fall_delay |= (1 << i);
> > + } else {
> > + fall_delay &= ~(1 << i);
> > + break;
> > + };
> > + }
> > }
> > final_fall_delay = get_best_delay(host, fall_delay);
> >
> > skip_fall:
> > final_maxlen = max(final_rise_delay.maxlen, final_fall_delay.maxlen);
> > + if (final_fall_delay.maxlen >= ENOUGH_MARGIN_MIN &&
> > + final_fall_delay.start < PREFER_START_POS_MAX)
> > + final_maxlen = final_fall_delay.maxlen;
> > if (final_maxlen == final_rise_delay.maxlen) {
> > sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> > - sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRDLY,
> > + sdr_set_field(host->base + MSDC_PAD_TUNE,
> > + MSDC_PAD_TUNE_CMDRDLY,
> > final_rise_delay.final_phase);
> > final_delay = final_rise_delay.final_phase;
> > } else {
> > sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> > - sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRDLY,
> > + sdr_set_field(host->base + MSDC_PAD_TUNE,
> > + MSDC_PAD_TUNE_CMDRDLY,
> > final_fall_delay.final_phase);
> > final_delay = final_fall_delay.final_phase;
> > }
> > + if (host->hs200_cmd_int_delay)
> > + goto skip_internal;
> >
> > + for (i = 0; i < PAD_DELAY_MAX; i++) {
> > + sdr_set_field(host->base + MSDC_PAD_TUNE,
> > + MSDC_PAD_TUNE_CMDRRDLY, i);
> > + mmc_send_tuning(mmc, opcode, &cmd_err);
> > + if (!cmd_err)
> > + internal_delay |= (1 << i);
> > + }
> > + dev_info(host->dev, "Final internal delay: 0x%x\n", internal_delay);
>
> I don't think dev_info() is what you want, right? Perhaps dev_dbg(),
> anything at all.
>
Yes. We should use dev_dbg() here. Will be fixed in next version.
> > + internal_delay_phase = get_best_delay(host, internal_delay);
> > + sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRRDLY,
> > + internal_delay_phase.final_phase);
> > +skip_internal:
> > + dev_info(host->dev, "Final cmd pad delay: %x\n", final_delay);
>
> I don't think dev_info() is what you want, right? Perhaps dev_dbg(),
> anything at all.
>
Yes. We should use dev_dbg() here. Will be fixed in next version.
> > + return final_delay == 0xff ? -EIO : 0;
> > +}
> > +
> > +static int hs400_tune_response(struct mmc_host *mmc, u32 opcode)
> > +{
> > + struct msdc_host *host = mmc_priv(mmc);
> > + u32 cmd_delay = 0;
> > + struct msdc_delay_phase final_cmd_delay = { 0,};
> > + u8 final_delay;
> > + int cmd_err;
> > + int i, j;
> > +
> > + /* select EMMC50 PAD CMD tune */
> > + sdr_set_bits(host->base + PAD_CMD_TUNE, BIT(0));
> > +
> > + if (mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
> > + mmc->ios.timing == MMC_TIMING_UHS_SDR104)
> > + sdr_set_field(host->base + MSDC_PAD_TUNE,
> > + MSDC_PAD_TUNE_CMDRRDLY,
> > + host->hs200_cmd_int_delay);
> > + sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_RSPL,
> > + host->hs200_cmd_resp_sel);
> > + for (i = 0 ; i < PAD_DELAY_MAX; i++) {
> > + sdr_set_field(host->base + PAD_CMD_TUNE,
> > + PAD_CMD_TUNE_RX_DLY3, i);
> > + for (j = 0; j < 3; j++) {
>
> 3?
>
> > + mmc_send_tuning(mmc, opcode, &cmd_err);
> > + if (!cmd_err) {
> > + cmd_delay |= (1 << i);
> > + } else {
> > + cmd_delay &= ~(1 << i);
> > + break;
> > + }
> > + }
> > + }
> > + final_cmd_delay = get_best_delay(host, cmd_delay);
> > + sdr_set_field(host->base + PAD_CMD_TUNE, PAD_CMD_TUNE_RX_DLY3,
> > + final_cmd_delay.final_phase);
> > + final_delay = final_cmd_delay.final_phase;
> > +
> > + dev_info(host->dev, "Final cmd pad delay: %x\n", final_delay);
>
> dev_info() -> dev_dbg() or remove it.
We will use dev_dbg() instead of dev_info(). Will be fixed in next
version.
>
> > return final_delay == 0xff ? -EIO : 0;
> > }
> >
> > @@ -1389,7 +1492,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
> > }
> > final_rise_delay = get_best_delay(host, rise_delay);
> > /* if rising edge has enough margin, then do not scan falling edge */
> > - if (final_rise_delay.maxlen >= 10 ||
> > + if (final_rise_delay.maxlen >= ENOUGH_MARGIN_MIN ||
>
> Clean up. Move to separate change.
>
Will drop it in current change.
> > (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
> > goto skip_fall;
> >
> > @@ -1422,6 +1525,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
> > final_delay = final_fall_delay.final_phase;
> > }
> >
> > + dev_info(host->dev, "Final data pad delay: %x\n", final_delay);
>
> dev_info() -> dev_dbg() or remove it.
>
We will use dev_dbg() instead of dev_info(). Will be fixed in next
version.
> > return final_delay == 0xff ? -EIO : 0;
> > }
> >
> > @@ -1430,10 +1534,13 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> > struct msdc_host *host = mmc_priv(mmc);
> > int ret;
> >
> > + if (host->hs400_mode)
> > + ret = hs400_tune_response(mmc, opcode);
> > + else
> > ret = msdc_tune_response(mmc, opcode);
>
> Because of the new else clause, seems like above needs an intendation.
>
Sorry. Will be fixed in next version.
> > if (ret == -EIO) {
> > dev_err(host->dev, "Tune response fail!\n");
> > - return ret;
> > + goto out;
>
> Not needed, remove the label and this change.
>
Will drop it in this change.
> > }
> > if (host->hs400_mode == false) {
> > ret = msdc_tune_data(mmc, opcode);
> > @@ -1443,6 +1550,8 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
> >
> > host->saved_tune_para.iocon = readl(host->base + MSDC_IOCON);
> > host->saved_tune_para.pad_tune = readl(host->base + MSDC_PAD_TUNE);
> > + host->saved_tune_para.pad_cmd_tune = readl(host->base + PAD_CMD_TUNE);
> > +out:
> > return ret;
> > }
> >
> > @@ -1553,6 +1662,18 @@ static int msdc_drv_probe(struct platform_device *pdev)
> > dev_dbg(&pdev->dev, "hs400-ds-delay: %x\n",
> > host->hs400_ds_delay);
> >
> > + if (!of_property_read_u32(pdev->dev.of_node, "hs200-cmd-int-delay",
> > + &host->hs200_cmd_int_delay))
> > + dev_dbg(&pdev->dev, "host->hs200-cmd-int-delay: %x\n",
> > + host->hs200_cmd_int_delay);
> > + if (!of_property_read_u32(pdev->dev.of_node, "hs400-cmd-int-delay",
> > + &host->hs400_cmd_int_delay))
> > + dev_dbg(&pdev->dev, "host->hs400-cmd-int-delay: %x\n",
> > + host->hs400_cmd_int_delay);
> > + if (!of_property_read_u32(pdev->dev.of_node, "cmd-resp-sel",
> > + &host->hs200_cmd_resp_sel))
> > + dev_dbg(&pdev->dev, "host->hs200_cmd-resp-sel: %x\n",
> > + host->hs200_cmd_resp_sel);
>
> I suggest you take the oppotunity to move the MTK DTS parsing into its
> own function, include the existing parsing of the "hs400-ds-delay".
> This improve the readablitity of the code.
>
Thanks. We will move all of MTK DTS parsing into msdc_of_property_parse
function.
> > host->dev = &pdev->dev;
> > host->mmc = mmc;
> > host->src_clk_freq = clk_get_rate(host->src_clk);
> > @@ -1663,6 +1784,7 @@ static void msdc_save_reg(struct msdc_host *host)
> > host->save_para.patch_bit0 = readl(host->base + MSDC_PATCH_BIT);
> > host->save_para.patch_bit1 = readl(host->base + MSDC_PATCH_BIT1);
> > host->save_para.pad_ds_tune = readl(host->base + PAD_DS_TUNE);
> > + host->save_para.pad_cmd_tune = readl(host->base + PAD_CMD_TUNE);
> > host->save_para.emmc50_cfg0 = readl(host->base + EMMC50_CFG0);
> > }
> >
> > @@ -1675,6 +1797,7 @@ static void msdc_restore_reg(struct msdc_host *host)
> > writel(host->save_para.patch_bit0, host->base + MSDC_PATCH_BIT);
> > writel(host->save_para.patch_bit1, host->base + MSDC_PATCH_BIT1);
> > writel(host->save_para.pad_ds_tune, host->base + PAD_DS_TUNE);
> > + writel(host->save_para.pad_cmd_tune, host->base + PAD_CMD_TUNE);
> > writel(host->save_para.emmc50_cfg0, host->base + EMMC50_CFG0);
> > }
> >
> > --
> > 1.7.9.5
> >
>
> Kind regards
> Uffe
Best Regards,
Yong Mao.
^ permalink raw reply
* [PATCH v2 0/5] fbdev/ssd1307fb: Some changes and improvement
From: Jyri Sarha @ 2017-01-13 10:35 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, tomi.valkeinen-l0cyMroinI0,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
bcousson-rdvid1DuHRBWk0Htik3J/w, Jyri Sarha
Changes since first version:
- Cherry-picked "fbdev/ssd1307fb: add support to enable VBAT" again
beacuse of weird mixup in the first version
- Also tested the code on top of v4.10-rc3 with am335x-icev2 evm
We needed these changes for couple of our evm. Some of these patches
have been on mainline list before, but after that forgotten. Now we
are forward porting these on for out internal release, so this is a
good time to get them to mainline too.
Everything else is quite straight forward, but removing the
reset-active-low dts property is a questionable at least in
theory. However, in practice if anyone was using the property in their
device-tree blobs, they have never been able to get desired effect
with mainline kernel.
Jyri Sarha (3):
fbdev: ssd1307fb: Start to use gpiod API for reset gpio
fbdev: ssd1307fb: Remove reset-active-low from the DT binding document
fbdev: ssd1307fb: Make reset gpio devicetree property optional
Tomi Valkeinen (2):
fbdev/ssd1307fb: add support to enable VBAT
fbdev/ssd1307fb: clear screen in probe
.../devicetree/bindings/display/ssd1307fb.txt | 5 +-
drivers/video/fbdev/ssd1307fb.c | 55 ++++++++++++++--------
2 files changed, 38 insertions(+), 22 deletions(-)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 1/5] fbdev: ssd1307fb: Start to use gpiod API for reset gpio
From: Jyri Sarha @ 2017-01-13 10:35 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, tomi.valkeinen-l0cyMroinI0,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
bcousson-rdvid1DuHRBWk0Htik3J/w, Jyri Sarha
In-Reply-To: <cover.1484303628.git.jsarha-l0cyMroinI0@public.gmane.org>
Start to use gpiod API for reset gpio.
Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
---
drivers/video/fbdev/ssd1307fb.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 2925d5c..8ffaaee 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -73,7 +73,7 @@ struct ssd1307fb_par {
u32 prechargep2;
struct pwm_device *pwm;
u32 pwm_period;
- int reset;
+ struct gpio_desc *reset;
u32 seg_remap;
u32 vcomh;
u32 width;
@@ -561,10 +561,11 @@ static int ssd1307fb_probe(struct i2c_client *client,
par->device_info = of_device_get_match_data(&client->dev);
- par->reset = of_get_named_gpio(client->dev.of_node,
- "reset-gpios", 0);
- if (!gpio_is_valid(par->reset)) {
- ret = -EINVAL;
+ par->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(par->reset)) {
+ dev_err(&client->dev, "failed to get reset gpio: %ld\n",
+ PTR_ERR(par->reset));
+ ret = PTR_ERR(par->reset);
goto fb_alloc_error;
}
@@ -642,22 +643,12 @@ static int ssd1307fb_probe(struct i2c_client *client,
fb_deferred_io_init(info);
- ret = devm_gpio_request_one(&client->dev, par->reset,
- GPIOF_OUT_INIT_HIGH,
- "oled-reset");
- if (ret) {
- dev_err(&client->dev,
- "failed to request gpio %d: %d\n",
- par->reset, ret);
- goto reset_oled_error;
- }
-
i2c_set_clientdata(client, info);
/* Reset the screen */
- gpio_set_value(par->reset, 0);
+ gpiod_set_value(par->reset, 0);
udelay(4);
- gpio_set_value(par->reset, 1);
+ gpiod_set_value(par->reset, 1);
udelay(4);
ret = ssd1307fb_init(par);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 2/5] fbdev: ssd1307fb: Remove reset-active-low from the DT binding document
From: Jyri Sarha @ 2017-01-13 10:35 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, tomi.valkeinen-l0cyMroinI0,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
bcousson-rdvid1DuHRBWk0Htik3J/w, Jyri Sarha
In-Reply-To: <cover.1484303628.git.jsarha-l0cyMroinI0@public.gmane.org>
Remove reset-active-low from the devicetree binding document. The actual
implementation has never been there in the driver code and there is no
reason to add it because the gpiod API supports gpio flags, including
GPIO_ACTIVE_LOW, directly trough its own devicetree binding.
Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
---
Documentation/devicetree/bindings/display/ssd1307fb.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index eb31ed4..4aee67f 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -8,14 +8,14 @@ Required properties:
0x3c or 0x3d
- pwm: Should contain the pwm to use according to the OF device tree PWM
specification [0]. Only required for the ssd1307.
- - reset-gpios: Should contain the GPIO used to reset the OLED display
+ - reset-gpios: Should contain the GPIO used to reset the OLED display. See
+ Documentation/devicetree/bindings/gpio/gpio.txt for details.
- solomon,height: Height in pixel of the screen driven by the controller
- solomon,width: Width in pixel of the screen driven by the controller
- solomon,page-offset: Offset of pages (band of 8 pixels) that the screen is
mapped to.
Optional properties:
- - reset-active-low: Is the reset gpio is active on physical low?
- solomon,segment-no-remap: Display needs normal (non-inverted) data column
to segment mapping
- solomon,com-seq: Display uses sequential COM pin configuration
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 3/5] fbdev: ssd1307fb: Make reset gpio devicetree property optional
From: Jyri Sarha @ 2017-01-13 10:35 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, tomi.valkeinen-l0cyMroinI0,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
bcousson-rdvid1DuHRBWk0Htik3J/w, Jyri Sarha
In-Reply-To: <cover.1484303628.git.jsarha-l0cyMroinI0@public.gmane.org>
Make reset gpio devicetree property optional. Depending on the board
designing there may not be a dedicated gpio for resetting the
display. Without a proper reset there may be some junk in the display
memory at probe time, so in such a case the display memory is cleared
before turning it on. The devicetree binding document is also updated.
Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
---
.../devicetree/bindings/display/ssd1307fb.txt | 4 ++--
drivers/video/fbdev/ssd1307fb.c | 19 +++++++++++++------
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index 4aee67f..6617df6 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -8,14 +8,14 @@ Required properties:
0x3c or 0x3d
- pwm: Should contain the pwm to use according to the OF device tree PWM
specification [0]. Only required for the ssd1307.
- - reset-gpios: Should contain the GPIO used to reset the OLED display. See
- Documentation/devicetree/bindings/gpio/gpio.txt for details.
- solomon,height: Height in pixel of the screen driven by the controller
- solomon,width: Width in pixel of the screen driven by the controller
- solomon,page-offset: Offset of pages (band of 8 pixels) that the screen is
mapped to.
Optional properties:
+ - reset-gpios: The GPIO used to reset the OLED display, if available. See
+ Documentation/devicetree/bindings/gpio/gpio.txt for details.
- solomon,segment-no-remap: Display needs normal (non-inverted) data column
to segment mapping
- solomon,com-seq: Display uses sequential COM pin configuration
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 8ffaaee..89372af 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -439,6 +439,10 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
if (ret < 0)
return ret;
+ /* Clear the screen if we could not give reset at probe time */
+ if (!par->reset)
+ ssd1307fb_update_display(par);
+
/* Turn on the display */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
if (ret < 0)
@@ -561,7 +565,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
par->device_info = of_device_get_match_data(&client->dev);
- par->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_LOW);
+ par->reset = devm_gpiod_get_optional(&client->dev, "reset",
+ GPIOD_OUT_LOW);
if (IS_ERR(par->reset)) {
dev_err(&client->dev, "failed to get reset gpio: %ld\n",
PTR_ERR(par->reset));
@@ -645,11 +650,13 @@ static int ssd1307fb_probe(struct i2c_client *client,
i2c_set_clientdata(client, info);
- /* Reset the screen */
- gpiod_set_value(par->reset, 0);
- udelay(4);
- gpiod_set_value(par->reset, 1);
- udelay(4);
+ if (par->reset) {
+ /* Reset the screen */
+ gpiod_set_value(par->reset, 0);
+ udelay(4);
+ gpiod_set_value(par->reset, 1);
+ udelay(4);
+ }
ret = ssd1307fb_init(par);
if (ret)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 4/5] fbdev/ssd1307fb: add support to enable VBAT
From: Jyri Sarha @ 2017-01-13 10:35 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, tomi.valkeinen-l0cyMroinI0,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
bcousson-rdvid1DuHRBWk0Htik3J/w, Jyri Sarha
In-Reply-To: <cover.1484303628.git.jsarha-l0cyMroinI0@public.gmane.org>
From: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
SSD1306 needs VBAT when it is wired in charge pump configuration. This
patch adds support to the driver to enable VBAT regulator at init time.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
Reviewed-by: Roger Quadros <rogerq-l0cyMroinI0@public.gmane.org>
Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
---
.../devicetree/bindings/display/ssd1307fb.txt | 1 +
drivers/video/fbdev/ssd1307fb.c | 20 +++++++++++++++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt b/Documentation/devicetree/bindings/display/ssd1307fb.txt
index 6617df6..209d931 100644
--- a/Documentation/devicetree/bindings/display/ssd1307fb.txt
+++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt
@@ -16,6 +16,7 @@ Required properties:
Optional properties:
- reset-gpios: The GPIO used to reset the OLED display, if available. See
Documentation/devicetree/bindings/gpio/gpio.txt for details.
+ - vbat-supply: The supply for VBAT
- solomon,segment-no-remap: Display needs normal (non-inverted) data column
to segment mapping
- solomon,com-seq: Display uses sequential COM pin configuration
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 89372af..616a6a3 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -16,6 +16,7 @@
#include <linux/of_gpio.h>
#include <linux/pwm.h>
#include <linux/uaccess.h>
+#include <linux/regulator/consumer.h>
#define SSD1307FB_DATA 0x40
#define SSD1307FB_COMMAND 0x80
@@ -74,6 +75,7 @@ struct ssd1307fb_par {
struct pwm_device *pwm;
u32 pwm_period;
struct gpio_desc *reset;
+ struct regulator *vbat_reg;
u32 seg_remap;
u32 vcomh;
u32 width;
@@ -574,6 +576,14 @@ static int ssd1307fb_probe(struct i2c_client *client,
goto fb_alloc_error;
}
+ par->vbat_reg = devm_regulator_get_optional(&client->dev, "vbat");
+ if (IS_ERR(par->vbat_reg)) {
+ dev_err(&client->dev, "failed to get VBAT regulator: %ld\n",
+ PTR_ERR(par->vbat_reg));
+ ret = PTR_ERR(par->vbat_reg);
+ goto fb_alloc_error;
+ }
+
if (of_property_read_u32(node, "solomon,width", &par->width))
par->width = 96;
@@ -658,9 +668,15 @@ static int ssd1307fb_probe(struct i2c_client *client,
udelay(4);
}
+ ret = regulator_enable(par->vbat_reg);
+ if (ret) {
+ dev_err(&client->dev, "failed to enable VBAT: %d\n", ret);
+ goto reset_oled_error;
+ }
+
ret = ssd1307fb_init(par);
if (ret)
- goto reset_oled_error;
+ goto regulator_enable_error;
ret = register_framebuffer(info);
if (ret) {
@@ -693,6 +709,8 @@ static int ssd1307fb_probe(struct i2c_client *client,
pwm_disable(par->pwm);
pwm_put(par->pwm);
};
+regulator_enable_error:
+ regulator_disable(par->vbat_reg);
reset_oled_error:
fb_deferred_io_cleanup(info);
fb_alloc_error:
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2 5/5] fbdev/ssd1307fb: clear screen in probe
From: Jyri Sarha @ 2017-01-13 10:35 UTC (permalink / raw)
To: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, tomi.valkeinen-l0cyMroinI0,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
bcousson-rdvid1DuHRBWk0Htik3J/w, Jyri Sarha
In-Reply-To: <cover.1484303628.git.jsarha-l0cyMroinI0@public.gmane.org>
From: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
SSD1306 does not clear the panel's framebuffer automatically, even if a
HW reset happens, so we need to do that at probe time before enabling
the panel.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
---
drivers/video/fbdev/ssd1307fb.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 616a6a3..5c87ae4 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -441,9 +441,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
if (ret < 0)
return ret;
- /* Clear the screen if we could not give reset at probe time */
- if (!par->reset)
- ssd1307fb_update_display(par);
+ /* Clear the screen */
+ ssd1307fb_update_display(par);
/* Turn on the display */
ret = ssd1307fb_write_cmd(par->client, SSD1307FB_DISPLAY_ON);
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v29 9/9] Documentation: dt: chosen properties for arm64 kdump
From: Mark Rutland @ 2017-01-13 11:17 UTC (permalink / raw)
To: AKASHI Takahiro, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
james.morse-5wv7dgnIgG8, geoff-wEGCiKHe2LqWVfeAwA7xHQ,
bauerman-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
dyoung-H+wXaHxf7aLQT0dZR+AlfA,
kexec-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170113091339.GK20972-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Fri, Jan 13, 2017 at 06:13:49PM +0900, AKASHI Takahiro wrote:
> On Thu, Jan 12, 2017 at 03:39:45PM +0000, Mark Rutland wrote:
> > On Wed, Dec 28, 2016 at 01:37:34PM +0900, AKASHI Takahiro wrote:
> > > +linux,crashkernel-base
> > > +linux,crashkernel-size
> > > +----------------------
> > > +
> > > +These properties (currently used on PowerPC and arm64) indicates
> > > +the base address and the size, respectively, of the reserved memory
> > > +range for crash dump kernel.
> >
> > From this description, it's not clear to me what the (expected)
> > consumers of this property are, nor what is expected to provide it.
> >
> > In previous rounds of review, I had assumed that this was used to
> > describe a preference to the first kernel as to what region of memory
> > should be used for a subsequent kdump kernel. Looking around, I'm not
> > sure if I was correct in that assessment.
> >
> > I see that arch/powerpc seems to consume this property to configure
> > crashk_res, but it also rewrites it based on crashk_res, presumably for
> > the benefit of userspace. It's not clear to me how on powerpc the kdump
> > kernel knows its memory range -- is more DT modification done in the
> > kernel and/or userspace?
>
> I don't believe that powerpc will rewrite the property any way.
> As far as I know from *the source code*, powerpc kernel retrieves
> the memory range for crash dump kernel from a kernel command line, i.e.
> crashkernel=, and then exposes it through DT to userspace (assuming
> kexec-tools).
The rewriting I describe is in export_crashk_values() in
arch/powerpc/kernel/machine_kexec.c, where the code deletes existing the
properties, and adds new ones, to the DT exposed to userspace.
So I think we're just quibbling over the definition of "rewrite".
> > arm64 we should either ensure that /proc/iomem is consistently usable
> > (and have userspace consistently use it), or we should expose a new file
> > specifically to expose this information.
>
> The thing that I had in my mind when adding this property is that
> /proc/iomem would be obsolete in the future, then we should have
> an alternative in hand.
Ok.
My disagreement is with using the DT as a channel to convey information
from the kernel to userspace.
I'm more than happy for a new file or other mechanism to express this
information. For example, we could add
/sys/kernel/kexec_crash_{base,size} or similar.
> > Further, I do not think we need this property. It makes more sense to me
> > for the preference of a a region to be described to the *first* kernel
> > using the command line consistently.
> >
> > So I think we should drop this property, and not use it on arm64. Please
> > document this as powerpc only.
>
> OK, but if we drop the property from arm64 code, we have no reason
> to leave its description in this patch.
> (In fact, there are a few more (undocumented) properties that only ppc
> uses for kdump.)
I'm happy to drop it, then.
> > > +linux,usable-memory-range
> > > +-------------------------
> > > +
> > > +This property (currently used only on arm64) holds the memory range,
> > > +the base address and the size, which can be used as system ram on
> > > +the *current* kernel. Note that, if this property is present, any memory
> > > +regions under "memory" nodes in DT blob or ones marked as "conventional
> > > +memory" in EFI memory map should be ignored.
> >
> > Could you please replace this with:
> >
> > This property (arm64 only) holds a base address and size, describing a
> > limited region in which memory may be considered available for use by
> > the kernel. Memory outside of this range is not available for use.
> >
> > This property describes a limitation: memory within this range is only
> > valid when also described through another mechanism that the kernel
> > would otherwise use to determine available memory (e.g. memory nodes
> > or the EFI memory map). Valid memory may be sparse within the range.
>
> Sure.
Cheers!
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 01/24] [media] dt-bindings: Add bindings for i.MX media driver
From: Philipp Zabel @ 2017-01-13 11:55 UTC (permalink / raw)
To: Steve Longerbeam
Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
tiffany.lin, jean-christophe.trotin, horms+renesas,
niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
andrew-ct.chen, gregkh, devicetree, linux-kernel,
linux-arm-kernel, linux-media, devel, Steve Longerbeam
In-Reply-To: <1483755102-24785-2-git-send-email-steve_longerbeam@mentor.com>
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> Add bindings documentation for the i.MX media driver.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> Documentation/devicetree/bindings/media/imx.txt | 57 +++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/imx.txt
>
> diff --git a/Documentation/devicetree/bindings/media/imx.txt b/Documentation/devicetree/bindings/media/imx.txt
> new file mode 100644
> index 0000000..254b64a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/imx.txt
> @@ -0,0 +1,57 @@
> +Freescale i.MX Media Video Devices
> +
> +Video Media Controller node
> +---------------------------
> +
> +This is the parent media controller node for video capture support.
> +
> +Required properties:
> +- compatible : "fsl,imx-media";
Would you be opposed to calling this "capture-subsystem" instead of
"imx-media"? We already use "fsl,imx-display-subsystem" and
"fsl,imx-gpu-subsystem" for the display and GPU compound devices.
> +- ports : Should contain a list of phandles pointing to camera
> + sensor interface ports of IPU devices
> +
> +
> +fim child node
> +--------------
> +
> +This is an optional child node of the ipu_csi port nodes. If present and
> +available, it enables the Frame Interval Monitor. Its properties can be
> +used to modify the method in which the FIM measures frame intervals.
> +Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
> +Frame Interval Monitor.
> +
> +Optional properties:
> +- fsl,input-capture-channel: an input capture channel and channel flags,
> + specified as <chan flags>. The channel number
> + must be 0 or 1. The flags can be
> + IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
> + IRQ_TYPE_EDGE_BOTH, and specify which input
> + capture signal edge will trigger the input
> + capture event. If an input capture channel is
> + specified, the FIM will use this method to
> + measure frame intervals instead of via the EOF
> + interrupt. The input capture method is much
> + preferred over EOF as it is not subject to
> + interrupt latency errors. However it requires
> + routing the VSYNC or FIELD output signals of
> + the camera sensor to one of the i.MX input
> + capture pads (SD1_DAT0, SD1_DAT1), which also
> + gives up support for SD1.
This is a clever method to get better frame timestamps. Too bad about
the routing requirements. Can this be used on Nitrogen6X?
> +
> +mipi_csi2 node
> +--------------
> +
> +This is the device node for the MIPI CSI-2 Receiver, required for MIPI
> +CSI-2 sensors.
> +
> +Required properties:
> +- compatible : "fsl,imx6-mipi-csi2";
I think this should get an additional "snps,dw-mipi-csi2" compatible,
since the only i.MX6 specific part is the bolted-on IPU2CSI gasket.
> +- reg : physical base address and length of the register set;
> +- clocks : the MIPI CSI-2 receiver requires three clocks: hsi_tx
> + (the DPHY clock), video_27m, and eim_sel;
Note that hsi_tx is incorrectly named. CCGR3[CG8] just happens to be the
shared gate bit that gates the HSI clocks as well as the MIPI
"ac_clk_125m", "cfg_clk", "ips_clk", and "pll_refclk" inputs to the mipi
csi-2 core, but we are missing shared gate clocks in the clock tree for
these.
Both cfg_clk and pll_refclk are sourced from video_27m, so "cfg" ->
video_27m seems fine.
But I don't get "dphy". Which input clock would that correspond to?
"pll_refclk?"
Also the pixel clock input is a gate after aclk_podf (which we call
eim_podf), not aclk_sel (eim_sel).
> +- clock-names : must contain "dphy", "cfg", "pix";
> +
> +Optional properties:
> +- interrupts : must contain two level-triggered interrupts,
> + in order: 100 and 101;
regards
Philipp
^ permalink raw reply
* Re: [PATCH v3 02/24] ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
From: Philipp Zabel @ 2017-01-13 11:57 UTC (permalink / raw)
To: Steve Longerbeam
Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
tiffany.lin, jean-christophe.trotin, horms+renesas,
niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
andrew-ct.chen, gregkh, devicetree, linux-kernel,
linux-arm-kernel, linux-media, devel, Steve Longerbeam
In-Reply-To: <1483755102-24785-3-git-send-email-steve_longerbeam@mentor.com>
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> Add to the MIPI CSI2 receiver node: compatible string, interrupt sources,
> clocks.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 53e6e63..42926e9 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -1125,7 +1125,14 @@
> };
>
> mipi_csi: mipi@021dc000 {
> + compatible = "fsl,imx6-mipi-csi2";
> reg = <0x021dc000 0x4000>;
> + interrupts = <0 100 0x04>, <0 101 0x04>;
> + clocks = <&clks IMX6QDL_CLK_HSI_TX>,
> + <&clks IMX6QDL_CLK_VIDEO_27M>,
> + <&clks IMX6QDL_CLK_EIM_SEL>;
I think the latter should be EIM_PODF
> + clock-names = "dphy", "cfg", "pix";
and I'm not sure dphy is the right name for this one. Is that the pll
ref input?
> + status = "disabled";
> };
>
> mipi_dsi: mipi@021e0000 {
regards
Philipp
^ permalink raw reply
* Re: [PATCH v3 05/24] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround
From: Philipp Zabel @ 2017-01-13 11:59 UTC (permalink / raw)
To: Steve Longerbeam
Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
tiffany.lin, jean-christophe.trotin, horms+renesas,
niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
andrew-ct.chen, gregkh, devicetree, linux-kernel,
linux-arm-kernel, linux-media, devel, Steve Longerbeam
In-Reply-To: <1483755102-24785-6-git-send-email-steve_longerbeam@mentor.com>
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> There is a pin conflict with GPIO_6. This pin functions as a power
> input pin to the OV5642 camera sensor, but ENET uses it as the h/w
> workaround for erratum ERR006687, to wake-up the ARM cores on normal
> RX and TX packet done events. So we need to remove the h/w workaround
> to support the OV5642. The result is that the CPUidle driver will no
> longer allow entering the deep idle states on the sabrelite.
>
> This is a partial revert of
>
> commit 6261c4c8f13e ("ARM: dts: imx6qdl-sabrelite: use GPIO_6 for FEC
> interrupt.")
> commit a28eeb43ee57 ("ARM: dts: imx6: tag boards that have the HW workaround
> for ERR006687")
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Pity this has to be removed for all, Nitrogen6X should really use DT
overlays for the add-on boards / cameras.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v3 06/24] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
From: Philipp Zabel @ 2017-01-13 12:03 UTC (permalink / raw)
To: Steve Longerbeam
Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
tiffany.lin, jean-christophe.trotin, horms+renesas,
niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
andrew-ct.chen, gregkh, devicetree, linux-kernel,
linux-arm-kernel, linux-media, devel, Steve Longerbeam
In-Reply-To: <1483755102-24785-7-git-send-email-steve_longerbeam@mentor.com>
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> Enables the OV5642 parallel-bus sensor, and the OV5640 MIPI CSI-2 sensor.
> Both hang off the same i2c2 bus, so they require different (and non-
> default) i2c slave addresses.
>
> The OV5642 connects to the parallel-bus mux input port on ipu1_csi0_mux.
>
> The OV5640 connects to the input port on the MIPI CSI-2 receiver on
> mipi_csi. It is set to transmit over MIPI virtual channel 1.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> arch/arm/boot/dts/imx6dl-sabrelite.dts | 5 ++
> arch/arm/boot/dts/imx6q-sabrelite.dts | 6 ++
> arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 118 +++++++++++++++++++++++++++++++
> 3 files changed, 129 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6dl-sabrelite.dts b/arch/arm/boot/dts/imx6dl-sabrelite.dts
> index 0f06ca5..fec2524 100644
> --- a/arch/arm/boot/dts/imx6dl-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6dl-sabrelite.dts
> @@ -48,3 +48,8 @@
> model = "Freescale i.MX6 DualLite SABRE Lite Board";
> compatible = "fsl,imx6dl-sabrelite", "fsl,imx6dl";
> };
> +
> +&ipu1_csi1_from_ipu1_csi1_mux {
> + data-lanes = <0 1>;
> + clock-lanes = <2>;
> +};
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index 66d10d8..9e2d26d 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -52,3 +52,9 @@
> &sata {
> status = "okay";
> };
> +
> +&ipu1_csi1_from_mipi_vc1 {
> + data-lanes = <0 1>;
> + clock-lanes = <2>;
> +};
> +
> diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> index 795b5a5..bca9fed 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
> @@ -39,6 +39,8 @@
> * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> * OTHER DEALINGS IN THE SOFTWARE.
> */
> +
> +#include <dt-bindings/clock/imx6qdl-clock.h>
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
>
> @@ -96,6 +98,15 @@
> };
> };
>
> + mipi_xclk: mipi_xclk {
> + compatible = "pwm-clock";
> + #clock-cells = <0>;
> + clock-frequency = <22000000>;
> + clock-output-names = "mipi_pwm3";
> + pwms = <&pwm3 0 45>; /* 1 / 45 ns = 22 MHz */
> + status = "okay";
> + };
> +
> gpio-keys {
> compatible = "gpio-keys";
> pinctrl-names = "default";
> @@ -220,6 +231,22 @@
> };
> };
>
> +&ipu1_csi0_from_ipu1_csi0_mux {
> + bus-width = <8>;
> + data-shift = <12>; /* Lines 19:12 used */
> + hsync-active = <1>;
> + vync-active = <1>;
> +};
> +
> +&ipu1_csi0_mux_from_parallel_sensor {
> + remote-endpoint = <&ov5642_to_ipu1_csi0_mux>;
> +};
> +
> +&ipu1_csi0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ipu1_csi0>;
> +};
> +
> &audmux {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_audmux>;
> @@ -299,6 +326,52 @@
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_i2c2>;
> status = "okay";
> +
> + ov5640: camera@40 {
> + compatible = "ovti,ov5640";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5640>;
> + clocks = <&mipi_xclk>;
> + clock-names = "xclk";
> + reg = <0x40>;
> + xclk = <22000000>;
This is superfluous, you can use clk_get_rate on mipi_xclk.
> + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>; /* NANDF_D5 */
> + pwdn-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* NANDF_WP_B */
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640_to_mipi_csi: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <&mipi_csi_from_mipi_sensor>;
> + data-lanes = <0 1>;
> + clock-lanes = <2>;
> + };
> + };
> + };
> +
> + ov5642: camera@42 {
> + compatible = "ovti,ov5642";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5642>;
> + clocks = <&clks IMX6QDL_CLK_CKO2>;
> + clock-names = "xclk";
> + reg = <0x42>;
> + xclk = <24000000>;
Same here, use assigned-clock-rates on IMX6QDL_CLK_CKO2 if necessary.
> + reset-gpios = <&gpio1 8 GPIO_ACTIVE_LOW>;
> + pwdn-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> + gp-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ov5642_to_ipu1_csi0_mux: endpoint {
> + remote-endpoint = <&ipu1_csi0_mux_from_parallel_sensor>;
> + bus-width = <8>;
> + hsync-active = <1>;
> + vsync-active = <1>;
> + };
> + };
> + };
> };
>
> &i2c3 {
> @@ -412,6 +485,23 @@
> >;
> };
>
> + pinctrl_ipu1_csi0: ipu1csi0grp {
> + fsl,pins = <
> + MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA12 0x1b0b0
> + MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA13 0x1b0b0
> + MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA14 0x1b0b0
> + MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA15 0x1b0b0
> + MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA16 0x1b0b0
> + MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA17 0x1b0b0
> + MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA18 0x1b0b0
> + MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA19 0x1b0b0
> + MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK 0x1b0b0
> + MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC 0x1b0b0
> + MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC 0x1b0b0
> + MX6QDL_PAD_CSI0_DATA_EN__IPU1_CSI0_DATA_EN 0x1b0b0
> + >;
> + };
> +
> pinctrl_j15: j15grp {
> fsl,pins = <
> MX6QDL_PAD_DI0_DISP_CLK__IPU1_DI0_DISP_CLK 0x10
> @@ -445,6 +535,22 @@
> >;
> };
>
> + pinctrl_ov5640: ov5640grp {
> + fsl,pins = <
> + MX6QDL_PAD_NANDF_D5__GPIO2_IO05 0x000b0
> + MX6QDL_PAD_NANDF_WP_B__GPIO6_IO09 0x0b0b0
> + >;
> + };
> +
> + pinctrl_ov5642: ov5642grp {
> + fsl,pins = <
> + MX6QDL_PAD_SD1_DAT0__GPIO1_IO16 0x1b0b0
> + MX6QDL_PAD_GPIO_6__GPIO1_IO06 0x1b0b0
> + MX6QDL_PAD_GPIO_8__GPIO1_IO08 0x130b0
> + MX6QDL_PAD_GPIO_3__CCM_CLKO2 0x000b0
> + >;
> + };
> +
> pinctrl_pwm1: pwm1grp {
> fsl,pins = <
> MX6QDL_PAD_SD1_DAT3__PWM1_OUT 0x1b0b1
> @@ -601,3 +707,15 @@
> vmmc-supply = <®_3p3v>;
> status = "okay";
> };
> +
> +&mipi_csi {
> + status = "okay";
> +};
> +
> +/* Incoming port from sensor */
> +&mipi_csi_from_mipi_sensor {
> + remote-endpoint = <&ov5640_to_mipi_csi>;
> + data-lanes = <0 1>;
> + clock-lanes = <2>;
> +};
> +
regards
Philipp
^ permalink raw reply
* Re: [PATCH v3] ARM: dts: Add LEGO MINDSTORMS EV3 dts
From: Sekhar Nori @ 2017-01-13 12:04 UTC (permalink / raw)
To: David Lechner, devicetree, linux-arm-kernel
Cc: Kevin Hilman, Rob Herring, Mark Rutland, linux-kernel
In-Reply-To: <1484253167-27568-1-git-send-email-david@lechnology.com>
On Friday 13 January 2017 02:02 AM, David Lechner wrote:
> This adds a device tree definition file for LEGO MINDSTORMS EV3.
>
> What is working:
>
> * Pin muxing
> * Pinconf
> * GPIOs
> * MicroSD card reader
> * UART on input port 1
> * Buttons
> * LEDs
> * Poweroff/reset
> * Flash memory
> * EEPROM
> * USB host port
> * USB peripheral port
>
> What is not working/to be added later:
>
> * Speaker - have patch submitted to get pwm-beeper working - maybe someday
> it will have a real sound driver that uses PRU
> * A/DC chip - have driver submitted and accepted - waiting for ack on
> device tree bindings
> * Display - waiting for "simple DRM" to be mainlined
> * Bluetooth - needs new driver for sequencing power/enable/clock
> * Input and output ports - need some sort of new phy or extcon driver as
> well as PRU UART and PRU I2C drivers
> * Battery indication - needs new power supply driver
>
> Note on flash partitions:
>
> These partitions are based on the official EV3 firmware from LEGO. It is
> expected that most users of the mainline kernel on EV3 will be booting from
> an SD card while retaining the official firmware in the flash memory.
> Furthermore, the official firmware uses an ancient U-Boot (2009) that has
> no device tree support. So, it makes sense to have this partition table in
> the EV3 device tree file. In the unlikely case that anyone does create their
> own firmware image with different partitioning, they can use a modern
> U-Boot in their own firmware image that modifies the device tree with the
> custom partitions.
>
> Signed-off-by: David Lechner <david@lechnology.com>
There are couple of checkpatch errors that show up. The compatible
"lego,ev3" needs to be documented in
Documentation/devicetree/bindings/arm/davinci.txt
"at24,24c128" is undocumented. Is that an atmel chip on the EV3? If the
manufacturer name is not clear,
Documentation/devicetree/bindings/eeprom/eeprom.txt advises using just
"24c128"
Finally, lego needs to be added to
Documentation/devicetree/bindings/vendor-prefixes.txt
Can you please submit the documentation portions as separate patches in
a series along with this patch.
Thanks,
Sekhar
^ permalink raw reply
* Re: [PATCH v3 15/24] media: Add userspace header file for i.MX
From: Philipp Zabel @ 2017-01-13 12:05 UTC (permalink / raw)
To: Steve Longerbeam
Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
mchehab, hverkuil, nick, markus.heiser, laurent.pinchart+renesas,
bparrot, geert, arnd, sudipm.mukherjee, minghsiu.tsai,
tiffany.lin, jean-christophe.trotin, horms+renesas,
niklas.soderlund+renesas, robert.jarzmik, songjun.wu,
andrew-ct.chen, gregkh, devicetree, linux-kernel,
linux-arm-kernel, linux-media, devel, Steve Longerbeam
In-Reply-To: <1483755102-24785-16-git-send-email-steve_longerbeam@mentor.com>
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> This adds a header file for use by userspace programs wanting to interact
> with the i.MX media driver. It defines custom v4l2 controls and events
> generated by the i.MX v4l2 subdevices.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> include/uapi/media/Kbuild | 1 +
> include/uapi/media/imx.h | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 31 insertions(+)
> create mode 100644 include/uapi/media/imx.h
>
> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
> index aafaa5a..fa78958 100644
> --- a/include/uapi/media/Kbuild
> +++ b/include/uapi/media/Kbuild
> @@ -1 +1,2 @@
> # UAPI Header export list
> +header-y += imx.h
> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
> new file mode 100644
> index 0000000..2421d9c
> --- /dev/null
> +++ b/include/uapi/media/imx.h
> @@ -0,0 +1,30 @@
> +/*
> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version
> + */
> +
> +#ifndef __UAPI_MEDIA_IMX_H__
> +#define __UAPI_MEDIA_IMX_H__
> +
> +/*
> + * events from the subdevs
> + */
> +#define V4L2_EVENT_IMX_CLASS V4L2_EVENT_PRIVATE_START
> +#define V4L2_EVENT_IMX_NFB4EOF (V4L2_EVENT_IMX_CLASS + 1)
> +#define V4L2_EVENT_IMX_EOF_TIMEOUT (V4L2_EVENT_IMX_CLASS + 2)
> +#define V4L2_EVENT_IMX_FRAME_INTERVAL (V4L2_EVENT_IMX_CLASS + 3)
Aren't these generic enough to warrant common events? I would think
there have to be other capture IP cores that can signal aborted frames
or frame timeouts.
> +
> +enum imx_ctrl_id {
> + V4L2_CID_IMX_MOTION = (V4L2_CID_USER_IMX_BASE + 0),
> + V4L2_CID_IMX_FIM_ENABLE,
> + V4L2_CID_IMX_FIM_NUM,
> + V4L2_CID_IMX_FIM_TOLERANCE_MIN,
> + V4L2_CID_IMX_FIM_TOLERANCE_MAX,
> + V4L2_CID_IMX_FIM_NUM_SKIP,
> +};
> +
> +#endif
regards
Philipp
^ permalink raw reply
* [PATCH v5 0/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Serge Semin @ 2017-01-13 12:16 UTC (permalink / raw)
To: gregkh, srinivas.kandagatla, andrew, robh+dt, mark.rutland
Cc: Sergey.Semin, linux-kernel, devicetree, Serge Semin
In-Reply-To: <1481638971-6247-1-git-send-email-fancer.lancer@gmail.com>
Changelog v3:
- Get rid of dev_*_idt() macros
- Replace to_pdev_kobj() macro with naked dev_get_drvdata() call
- Return naked 0 instead of SUCCESS macro
- IDT CSR debug file is moved to debugfs
- BIN_ATTR_RW is used to declare sysfs binary attribute
- Moved bindings file to a separate patch
- Need to create a specific bin_attribute structure for each device
- Perform a few read retries with delays if EEPROM is busy
Changelog v4:
- Make 89HPESx device bindings to have one EEPROM subnode
- Alter 89HPESx device bindings text
Changelog v5:
- Replace atomic_t CSR field with of u16 type
- Get rid of dev_dbg() print outs of initialization methods
- Simplify DebugFS initialization method
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
Serge Semin (2):
eeprom: Add IDT 89HPESx EEPROM/CSR driver
eeprom: Add IDT 89HPESx driver bindings file
.../devicetree/bindings/misc/idt_89hpesx.txt | 44 +
drivers/misc/eeprom/Kconfig | 10 +
drivers/misc/eeprom/Makefile | 1 +
drivers/misc/eeprom/idt_89hpesx.c | 1587 ++++++++++++++++++++
4 files changed, 1642 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt
create mode 100644 drivers/misc/eeprom/idt_89hpesx.c
--
2.6.6
^ permalink raw reply
* [PATCH v5 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
From: Serge Semin @ 2017-01-13 12:16 UTC (permalink / raw)
To: gregkh, srinivas.kandagatla, andrew, robh+dt, mark.rutland
Cc: Sergey.Semin, linux-kernel, devicetree, Serge Semin
In-Reply-To: <1484309813-25008-1-git-send-email-fancer.lancer@gmail.com>
This driver provides an access to EEPROM of IDT PCIe-switches. IDT PCIe-
switches expose a simple SMBus interface to perform IO-operations from/to
EEPROM, which is located at private (so called Master) SMBus. The driver
creates a simple binary sysfs-file to have an access to the EEPROM using
the SMBus-slave interface in the i2c-device susfs-directory:
/sys/bus/i2c/devices/<bus>-<devaddr>/eeprom
In case if read-only flag is specified at dts-node of the device, User-space
applications won't be able to write to the EEPROM sysfs-node.
Additionally IDT 89HPESx SMBus interface has an ability to read/write
values of device CSRs. This driver exposes debugfs-file to perform simple
IO-operations using that ability for just basic debug purpose. Particularly
the next file is created in the specific debugfs-directory:
/sys/kernel/debug/idt_csr/
Format of the debugfs-file value is:
$ cat /sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>;
<CSR address>:<CSR value>
So reading the content of the file gives current CSR address and it value.
If User-space application wishes to change current CSR address, it can just
write a proper value to the sysfs-file:
$ echo "<CSR address>" >
/sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>
If it wants to change the CSR value as well, the format of the write
operation is:
$ echo "<CSR address>:<CSR value>" > \
/sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>;
CSR address and value can be any of hexadecimal, decimal or octal format.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
drivers/misc/eeprom/Kconfig | 10 +
drivers/misc/eeprom/Makefile | 1 +
drivers/misc/eeprom/idt_89hpesx.c | 1587 +++++++++++++++++++++++++++++++++++++
3 files changed, 1598 insertions(+)
create mode 100644 drivers/misc/eeprom/idt_89hpesx.c
diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig
index c4e41c2..de58762 100644
--- a/drivers/misc/eeprom/Kconfig
+++ b/drivers/misc/eeprom/Kconfig
@@ -100,4 +100,14 @@ config EEPROM_DIGSY_MTC_CFG
If unsure, say N.
+config EEPROM_IDT_89HPESX
+ tristate "IDT 89HPESx PCIe-swtiches EEPROM / CSR support"
+ depends on I2C && SYSFS
+ help
+ Enable this driver to get read/write access to EEPROM / CSRs
+ over IDT PCIe-swtich i2c-slave interface.
+
+ This driver can also be built as a module. If so, the module
+ will be called idt_89hpesx.
+
endmenu
diff --git a/drivers/misc/eeprom/Makefile b/drivers/misc/eeprom/Makefile
index fc1e81d..90a5262 100644
--- a/drivers/misc/eeprom/Makefile
+++ b/drivers/misc/eeprom/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_EEPROM_MAX6875) += max6875.o
obj-$(CONFIG_EEPROM_93CX6) += eeprom_93cx6.o
obj-$(CONFIG_EEPROM_93XX46) += eeprom_93xx46.o
obj-$(CONFIG_EEPROM_DIGSY_MTC_CFG) += digsy_mtc_eeprom.o
+obj-$(CONFIG_EEPROM_IDT_89HPESX) += idt_89hpesx.o
diff --git a/drivers/misc/eeprom/idt_89hpesx.c b/drivers/misc/eeprom/idt_89hpesx.c
new file mode 100644
index 0000000..c53f0e5
--- /dev/null
+++ b/drivers/misc/eeprom/idt_89hpesx.c
@@ -0,0 +1,1587 @@
+/*
+ * This file is provided under a GPLv2 license. When using or
+ * redistributing this file, you may do so under that license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright (C) 2016 T-Platforms. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, it can be found <http://www.gnu.org/licenses/>.
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * IDT PCIe-switch NTB Linux driver
+ *
+ * Contact Information:
+ * Serge Semin <fancer.lancer@gmail.com>, <Sergey.Semin@t-platforms.ru>
+ */
+/*
+ * NOTE of the IDT 89HPESx SMBus-slave interface driver
+ * This driver primarily is developed to have an access to EEPROM device of
+ * IDT PCIe-switches. IDT provides a simple SMBus interface to perform IO-
+ * operations from/to EEPROM, which is located at private (so called Master)
+ * SMBus of switches. Using that interface this the driver creates a simple
+ * binary sysfs-file in the device directory:
+ * /sys/bus/i2c/devices/<bus>-<devaddr>/eeprom
+ * In case if read-only flag is specified in the dts-node of device desription,
+ * User-space applications won't be able to write to the EEPROM sysfs-node.
+ * Additionally IDT 89HPESx SMBus interface has an ability to write/read
+ * data of device CSRs. This driver exposes debugf-file to perform simple IO
+ * operations using that ability for just basic debug purpose. Particularly
+ * next file is created in the specific debugfs-directory:
+ * /sys/kernel/debug/idt_csr/
+ * Format of the debugfs-node is:
+ * $ cat /sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>;
+ * <CSR address>:<CSR value>
+ * So reading the content of the file gives current CSR address and it value.
+ * If User-space application wishes to change current CSR address,
+ * it can just write a proper value to the sysfs-file:
+ * $ echo "<CSR address>" > /sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>
+ * If it wants to change the CSR value as well, the format of the write
+ * operation is:
+ * $ echo "<CSR address>:<CSR value>" > \
+ * /sys/kernel/debug/idt_csr/<bus>-<devaddr>/<devname>;
+ * CSR address and value can be any of hexadecimal, decimal or octal format.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/sysfs.h>
+#include <linux/debugfs.h>
+#include <linux/mod_devicetable.h>
+#include <linux/of.h>
+#include <linux/i2c.h>
+#include <linux/pci_ids.h>
+#include <linux/delay.h>
+
+#define IDT_NAME "89hpesx"
+#define IDT_89HPESX_DESC "IDT 89HPESx SMBus-slave interface driver"
+#define IDT_89HPESX_VER "1.0"
+
+MODULE_DESCRIPTION(IDT_89HPESX_DESC);
+MODULE_VERSION(IDT_89HPESX_VER);
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("T-platforms");
+
+/*
+ * csr_dbgdir - CSR read/write operations Debugfs directory
+ */
+static struct dentry *csr_dbgdir;
+
+/*
+ * struct idt_89hpesx_dev - IDT 89HPESx device data structure
+ * @eesize: Size of EEPROM in bytes (calculated from "idt,eecompatible")
+ * @eero: EEPROM Read-only flag
+ * @eeaddr: EEPROM custom address
+ *
+ * @inieecmd: Initial cmd value for EEPROM read/write operations
+ * @inicsrcmd: Initial cmd value for CSR read/write operations
+ * @iniccode: Initialial command code value for IO-operations
+ *
+ * @csr: CSR address to perform read operation
+ *
+ * @smb_write: SMBus write method
+ * @smb_read: SMBus read method
+ * @smb_mtx: SMBus mutex
+ *
+ * @client: i2c client used to perform IO operations
+ *
+ * @ee_file: EEPROM read/write sysfs-file
+ * @csr_file: CSR read/write debugfs-node
+ */
+struct idt_smb_seq;
+struct idt_89hpesx_dev {
+ u32 eesize;
+ bool eero;
+ u8 eeaddr;
+
+ u8 inieecmd;
+ u8 inicsrcmd;
+ u8 iniccode;
+
+ u16 csr;
+
+ int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *);
+ int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *);
+ struct mutex smb_mtx;
+
+ struct i2c_client *client;
+
+ struct bin_attribute *ee_file;
+ struct dentry *csr_dir;
+ struct dentry *csr_file;
+};
+
+/*
+ * struct idt_smb_seq - sequence of data to be read/written from/to IDT 89HPESx
+ * @ccode: SMBus command code
+ * @bytecnt: Byte count of operation
+ * @data: Data to by written
+ */
+struct idt_smb_seq {
+ u8 ccode;
+ u8 bytecnt;
+ u8 *data;
+};
+
+/*
+ * struct idt_eeprom_seq - sequence of data to be read/written from/to EEPROM
+ * @cmd: Transaction CMD
+ * @eeaddr: EEPROM custom address
+ * @memaddr: Internal memory address of EEPROM
+ * @data: Data to be written at the memory address
+ */
+struct idt_eeprom_seq {
+ u8 cmd;
+ u8 eeaddr;
+ u16 memaddr;
+ u8 data;
+} __packed;
+
+/*
+ * struct idt_csr_seq - sequence of data to be read/written from/to CSR
+ * @cmd: Transaction CMD
+ * @csraddr: Internal IDT device CSR address
+ * @data: Data to be read/written from/to the CSR address
+ */
+struct idt_csr_seq {
+ u8 cmd;
+ u16 csraddr;
+ u32 data;
+} __packed;
+
+/*
+ * SMBus command code macros
+ * @CCODE_END: Indicates the end of transaction
+ * @CCODE_START: Indicates the start of transaction
+ * @CCODE_CSR: CSR read/write transaction
+ * @CCODE_EEPROM: EEPROM read/write transaction
+ * @CCODE_BYTE: Supplied data has BYTE length
+ * @CCODE_WORD: Supplied data has WORD length
+ * @CCODE_BLOCK: Supplied data has variable length passed in bytecnt
+ * byte right following CCODE byte
+ */
+#define CCODE_END ((u8)0x01)
+#define CCODE_START ((u8)0x02)
+#define CCODE_CSR ((u8)0x00)
+#define CCODE_EEPROM ((u8)0x04)
+#define CCODE_BYTE ((u8)0x00)
+#define CCODE_WORD ((u8)0x20)
+#define CCODE_BLOCK ((u8)0x40)
+#define CCODE_PEC ((u8)0x80)
+
+/*
+ * EEPROM command macros
+ * @EEPROM_OP_WRITE: EEPROM write operation
+ * @EEPROM_OP_READ: EEPROM read operation
+ * @EEPROM_USA: Use specified address of EEPROM
+ * @EEPROM_NAERR: EEPROM device is not ready to respond
+ * @EEPROM_LAERR: EEPROM arbitration loss error
+ * @EEPROM_MSS: EEPROM misplace start & stop bits error
+ * @EEPROM_WR_CNT: Bytes count to perform write operation
+ * @EEPROM_WRRD_CNT: Bytes count to write before reading
+ * @EEPROM_RD_CNT: Bytes count to perform read operation
+ * @EEPROM_DEF_SIZE: Fall back size of EEPROM
+ * @EEPROM_DEF_ADDR: Defatul EEPROM address
+ * @EEPROM_TOUT: Timeout before retry read operation if eeprom is busy
+ */
+#define EEPROM_OP_WRITE ((u8)0x00)
+#define EEPROM_OP_READ ((u8)0x01)
+#define EEPROM_USA ((u8)0x02)
+#define EEPROM_NAERR ((u8)0x08)
+#define EEPROM_LAERR ((u8)0x10)
+#define EEPROM_MSS ((u8)0x20)
+#define EEPROM_WR_CNT ((u8)5)
+#define EEPROM_WRRD_CNT ((u8)4)
+#define EEPROM_RD_CNT ((u8)5)
+#define EEPROM_DEF_SIZE ((u16)4096)
+#define EEPROM_DEF_ADDR ((u8)0x50)
+#define EEPROM_TOUT (100)
+
+/*
+ * CSR command macros
+ * @CSR_DWE: Enable all four bytes of the operation
+ * @CSR_OP_WRITE: CSR write operation
+ * @CSR_OP_READ: CSR read operation
+ * @CSR_RERR: Read operation error
+ * @CSR_WERR: Write operation error
+ * @CSR_WR_CNT: Bytes count to perform write operation
+ * @CSR_WRRD_CNT: Bytes count to write before reading
+ * @CSR_RD_CNT: Bytes count to perform read operation
+ * @CSR_MAX: Maximum CSR address
+ * @CSR_DEF: Default CSR address
+ * @CSR_REAL_ADDR: CSR real unshifted address
+ */
+#define CSR_DWE ((u8)0x0F)
+#define CSR_OP_WRITE ((u8)0x00)
+#define CSR_OP_READ ((u8)0x10)
+#define CSR_RERR ((u8)0x40)
+#define CSR_WERR ((u8)0x80)
+#define CSR_WR_CNT ((u8)7)
+#define CSR_WRRD_CNT ((u8)3)
+#define CSR_RD_CNT ((u8)7)
+#define CSR_MAX ((u32)0x3FFFF)
+#define CSR_DEF ((u16)0x0000)
+#define CSR_REAL_ADDR(val) ((unsigned int)val << 2)
+
+/*
+ * IDT 89HPESx basic register
+ * @IDT_VIDDID_CSR: PCIe VID and DID of IDT 89HPESx
+ * @IDT_VID_MASK: Mask of VID
+ */
+#define IDT_VIDDID_CSR ((u32)0x0000)
+#define IDT_VID_MASK ((u32)0xFFFF)
+
+/*
+ * IDT 89HPESx can send NACK when new command is sent before previous one
+ * fininshed execution. In this case driver retries operation
+ * certain times.
+ * @RETRY_CNT: Number of retries before giving up and fail
+ * @idt_smb_safe: Generate a retry loop on corresponding SMBus method
+ */
+#define RETRY_CNT (128)
+#define idt_smb_safe(ops, args...) ({ \
+ int __retry = RETRY_CNT; \
+ s32 __sts; \
+ do { \
+ __sts = i2c_smbus_ ## ops ## _data(args); \
+ } while (__retry-- && __sts < 0); \
+ __sts; \
+})
+
+/*===========================================================================
+ * i2c bus level IO-operations
+ *===========================================================================
+ */
+
+/*
+ * idt_smb_write_byte() - SMBus write method when I2C_SMBUS_BYTE_DATA operation
+ * is only available
+ * @pdev: Pointer to the driver data
+ * @seq: Sequence of data to be written
+ */
+static int idt_smb_write_byte(struct idt_89hpesx_dev *pdev,
+ const struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+ int idx;
+
+ /* Loop over the supplied data sending byte one-by-one */
+ for (idx = 0; idx < seq->bytecnt; idx++) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BYTE;
+ if (idx == 0)
+ ccode |= CCODE_START;
+ if (idx == seq->bytecnt - 1)
+ ccode |= CCODE_END;
+
+ /* Send data to the device */
+ sts = idt_smb_safe(write_byte, pdev->client, ccode,
+ seq->data[idx]);
+ if (sts != 0)
+ return (int)sts;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_smb_read_byte() - SMBus read method when I2C_SMBUS_BYTE_DATA operation
+ * is only available
+ * @pdev: Pointer to the driver data
+ * @seq: Buffer to read data to
+ */
+static int idt_smb_read_byte(struct idt_89hpesx_dev *pdev,
+ struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+ int idx;
+
+ /* Loop over the supplied buffer receiving byte one-by-one */
+ for (idx = 0; idx < seq->bytecnt; idx++) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BYTE;
+ if (idx == 0)
+ ccode |= CCODE_START;
+ if (idx == seq->bytecnt - 1)
+ ccode |= CCODE_END;
+
+ /* Read data from the device */
+ sts = idt_smb_safe(read_byte, pdev->client, ccode);
+ if (sts < 0)
+ return (int)sts;
+
+ seq->data[idx] = (u8)sts;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_smb_write_word() - SMBus write method when I2C_SMBUS_BYTE_DATA and
+ * I2C_FUNC_SMBUS_WORD_DATA operations are available
+ * @pdev: Pointer to the driver data
+ * @seq: Sequence of data to be written
+ */
+static int idt_smb_write_word(struct idt_89hpesx_dev *pdev,
+ const struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+ int idx, evencnt;
+
+ /* Calculate the even count of data to send */
+ evencnt = seq->bytecnt - (seq->bytecnt % 2);
+
+ /* Loop over the supplied data sending two bytes at a time */
+ for (idx = 0; idx < evencnt; idx += 2) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_WORD;
+ if (idx == 0)
+ ccode |= CCODE_START;
+ if (idx == evencnt - 2)
+ ccode |= CCODE_END;
+
+ /* Send word data to the device */
+ sts = idt_smb_safe(write_word, pdev->client, ccode,
+ *(u16 *)&seq->data[idx]);
+ if (sts != 0)
+ return (int)sts;
+ }
+
+ /* If there is odd number of bytes then send just one last byte */
+ if (seq->bytecnt != evencnt) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BYTE | CCODE_END;
+ if (idx == 0)
+ ccode |= CCODE_START;
+
+ /* Send byte data to the device */
+ sts = idt_smb_safe(write_byte, pdev->client, ccode,
+ seq->data[idx]);
+ if (sts != 0)
+ return (int)sts;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_smb_read_word() - SMBus read method when I2C_SMBUS_BYTE_DATA and
+ * I2C_FUNC_SMBUS_WORD_DATA operations are available
+ * @pdev: Pointer to the driver data
+ * @seq: Buffer to read data to
+ */
+static int idt_smb_read_word(struct idt_89hpesx_dev *pdev,
+ struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+ int idx, evencnt;
+
+ /* Calculate the even count of data to send */
+ evencnt = seq->bytecnt - (seq->bytecnt % 2);
+
+ /* Loop over the supplied data reading two bytes at a time */
+ for (idx = 0; idx < evencnt; idx += 2) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_WORD;
+ if (idx == 0)
+ ccode |= CCODE_START;
+ if (idx == evencnt - 2)
+ ccode |= CCODE_END;
+
+ /* Read word data from the device */
+ sts = idt_smb_safe(read_word, pdev->client, ccode);
+ if (sts < 0)
+ return (int)sts;
+
+ *(u16 *)&seq->data[idx] = (u16)sts;
+ }
+
+ /* If there is odd number of bytes then receive just one last byte */
+ if (seq->bytecnt != evencnt) {
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BYTE | CCODE_END;
+ if (idx == 0)
+ ccode |= CCODE_START;
+
+ /* Read last data byte from the device */
+ sts = idt_smb_safe(read_byte, pdev->client, ccode);
+ if (sts < 0)
+ return (int)sts;
+
+ seq->data[idx] = (u8)sts;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_smb_write_block() - SMBus write method when I2C_SMBUS_BLOCK_DATA
+ * operation is available
+ * @pdev: Pointer to the driver data
+ * @seq: Sequence of data to be written
+ */
+static int idt_smb_write_block(struct idt_89hpesx_dev *pdev,
+ const struct idt_smb_seq *seq)
+{
+ u8 ccode;
+
+ /* Return error if too much data passed to send */
+ if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
+
+ /* Send block of data to the device */
+ return idt_smb_safe(write_block, pdev->client, ccode, seq->bytecnt,
+ seq->data);
+}
+
+/*
+ * idt_smb_read_block() - SMBus read method when I2C_SMBUS_BLOCK_DATA
+ * operation is available
+ * @pdev: Pointer to the driver data
+ * @seq: Buffer to read data to
+ */
+static int idt_smb_read_block(struct idt_89hpesx_dev *pdev,
+ struct idt_smb_seq *seq)
+{
+ s32 sts;
+ u8 ccode;
+
+ /* Return error if too much data passed to send */
+ if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
+
+ /* Read block of data from the device */
+ sts = idt_smb_safe(read_block, pdev->client, ccode, seq->data);
+ if (sts != seq->bytecnt)
+ return (sts < 0 ? sts : -ENODATA);
+
+ return 0;
+}
+
+/*
+ * idt_smb_write_i2c_block() - SMBus write method when I2C_SMBUS_I2C_BLOCK_DATA
+ * operation is available
+ * @pdev: Pointer to the driver data
+ * @seq: Sequence of data to be written
+ *
+ * NOTE It's usual SMBus write block operation, except the actual data length is
+ * sent as first byte of data
+ */
+static int idt_smb_write_i2c_block(struct idt_89hpesx_dev *pdev,
+ const struct idt_smb_seq *seq)
+{
+ u8 ccode, buf[I2C_SMBUS_BLOCK_MAX + 1];
+
+ /* Return error if too much data passed to send */
+ if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ /* Collect the data to send. Length byte must be added prior the data */
+ buf[0] = seq->bytecnt;
+ memcpy(&buf[1], seq->data, seq->bytecnt);
+
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
+
+ /* Send length and block of data to the device */
+ return idt_smb_safe(write_i2c_block, pdev->client, ccode,
+ seq->bytecnt + 1, buf);
+}
+
+/*
+ * idt_smb_read_i2c_block() - SMBus read method when I2C_SMBUS_I2C_BLOCK_DATA
+ * operation is available
+ * @pdev: Pointer to the driver data
+ * @seq: Buffer to read data to
+ *
+ * NOTE It's usual SMBus read block operation, except the actual data length is
+ * retrieved as first byte of data
+ */
+static int idt_smb_read_i2c_block(struct idt_89hpesx_dev *pdev,
+ struct idt_smb_seq *seq)
+{
+ u8 ccode, buf[I2C_SMBUS_BLOCK_MAX + 1];
+ s32 sts;
+
+ /* Return error if too much data passed to send */
+ if (seq->bytecnt > I2C_SMBUS_BLOCK_MAX)
+ return -EINVAL;
+
+ /* Collect the command code byte */
+ ccode = seq->ccode | CCODE_BLOCK | CCODE_START | CCODE_END;
+
+ /* Read length and block of data from the device */
+ sts = idt_smb_safe(read_i2c_block, pdev->client, ccode,
+ seq->bytecnt + 1, buf);
+ if (sts != seq->bytecnt + 1)
+ return (sts < 0 ? sts : -ENODATA);
+ if (buf[0] != seq->bytecnt)
+ return -ENODATA;
+
+ /* Copy retrieved data to the output data buffer */
+ memcpy(seq->data, &buf[1], seq->bytecnt);
+
+ return 0;
+}
+
+/*===========================================================================
+ * EEPROM IO-operations
+ *===========================================================================
+ */
+
+/*
+ * idt_eeprom_read_byte() - read just one byte from EEPROM
+ * @pdev: Pointer to the driver data
+ * @memaddr: Start EEPROM memory address
+ * @data: Data to be written to EEPROM
+ */
+static int idt_eeprom_read_byte(struct idt_89hpesx_dev *pdev, u16 memaddr,
+ u8 *data)
+{
+ struct device *dev = &pdev->client->dev;
+ struct idt_eeprom_seq eeseq;
+ struct idt_smb_seq smbseq;
+ int ret, retry;
+
+ /* Initialize SMBus sequence fields */
+ smbseq.ccode = pdev->iniccode | CCODE_EEPROM;
+ smbseq.data = (u8 *)&eeseq;
+
+ /*
+ * Sometimes EEPROM may respond with NACK if it's busy with previous
+ * operation, so we need to perform a few attempts of read cycle
+ */
+ retry = RETRY_CNT;
+ do {
+ /* Send EEPROM memory address to read data from */
+ smbseq.bytecnt = EEPROM_WRRD_CNT;
+ eeseq.cmd = pdev->inieecmd | EEPROM_OP_READ;
+ eeseq.eeaddr = pdev->eeaddr;
+ eeseq.memaddr = cpu_to_le16(memaddr);
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to init eeprom addr 0x%02hhx",
+ memaddr);
+ break;
+ }
+
+ /* Perform read operation */
+ smbseq.bytecnt = EEPROM_RD_CNT;
+ ret = pdev->smb_read(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to read eeprom data 0x%02hhx",
+ memaddr);
+ break;
+ }
+
+ /* Restart read operation if the device is busy */
+ if (retry && (eeseq.cmd & EEPROM_NAERR)) {
+ dev_dbg(dev, "EEPROM busy, retry reading after %d ms",
+ EEPROM_TOUT);
+ msleep(EEPROM_TOUT);
+ continue;
+ }
+
+ /* Check whether IDT successfully read data from EEPROM */
+ if (eeseq.cmd & (EEPROM_NAERR | EEPROM_LAERR | EEPROM_MSS)) {
+ dev_err(dev,
+ "Communication with eeprom failed, cmd 0x%hhx",
+ eeseq.cmd);
+ ret = -EREMOTEIO;
+ break;
+ }
+
+ /* Save retrieved data and exit the loop */
+ *data = eeseq.data;
+ break;
+ } while (retry--);
+
+ /* Return the status of operation */
+ return ret;
+}
+
+/*
+ * idt_eeprom_write() - EEPROM write operation
+ * @pdev: Pointer to the driver data
+ * @memaddr: Start EEPROM memory address
+ * @len: Length of data to be written
+ * @data: Data to be written to EEPROM
+ */
+static int idt_eeprom_write(struct idt_89hpesx_dev *pdev, u16 memaddr, u16 len,
+ const u8 *data)
+{
+ struct device *dev = &pdev->client->dev;
+ struct idt_eeprom_seq eeseq;
+ struct idt_smb_seq smbseq;
+ int ret;
+ u16 idx;
+
+ /* Initialize SMBus sequence fields */
+ smbseq.ccode = pdev->iniccode | CCODE_EEPROM;
+ smbseq.data = (u8 *)&eeseq;
+
+ /* Send data byte-by-byte, checking if it is successfully written */
+ for (idx = 0; idx < len; idx++, memaddr++) {
+ /* Lock IDT SMBus device */
+ mutex_lock(&pdev->smb_mtx);
+
+ /* Perform write operation */
+ smbseq.bytecnt = EEPROM_WR_CNT;
+ eeseq.cmd = pdev->inieecmd | EEPROM_OP_WRITE;
+ eeseq.eeaddr = pdev->eeaddr;
+ eeseq.memaddr = cpu_to_le16(memaddr);
+ eeseq.data = data[idx];
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev,
+ "Failed to write 0x%04hx:0x%02hhx to eeprom",
+ memaddr, data[idx]);
+ goto err_mutex_unlock;
+ }
+
+ /*
+ * Check whether the data is successfully written by reading
+ * from the same EEPROM memory address.
+ */
+ eeseq.data = ~data[idx];
+ ret = idt_eeprom_read_byte(pdev, memaddr, &eeseq.data);
+ if (ret != 0)
+ goto err_mutex_unlock;
+
+ /* Check whether the read byte is the same as written one */
+ if (eeseq.data != data[idx]) {
+ dev_err(dev, "Values don't match 0x%02hhx != 0x%02hhx",
+ eeseq.data, data[idx]);
+ ret = -EREMOTEIO;
+ goto err_mutex_unlock;
+ }
+
+ /* Unlock IDT SMBus device */
+err_mutex_unlock:
+ mutex_unlock(&pdev->smb_mtx);
+ if (ret != 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_eeprom_read() - EEPROM read operation
+ * @pdev: Pointer to the driver data
+ * @memaddr: Start EEPROM memory address
+ * @len: Length of data to read
+ * @buf: Buffer to read data to
+ */
+static int idt_eeprom_read(struct idt_89hpesx_dev *pdev, u16 memaddr, u16 len,
+ u8 *buf)
+{
+ int ret;
+ u16 idx;
+
+ /* Read data byte-by-byte, retrying if it wasn't successful */
+ for (idx = 0; idx < len; idx++, memaddr++) {
+ /* Lock IDT SMBus device */
+ mutex_lock(&pdev->smb_mtx);
+
+ /* Just read the byte to the buffer */
+ ret = idt_eeprom_read_byte(pdev, memaddr, &buf[idx]);
+
+ /* Unlock IDT SMBus device */
+ mutex_unlock(&pdev->smb_mtx);
+
+ /* Return error if read operation failed */
+ if (ret != 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+/*===========================================================================
+ * CSR IO-operations
+ *===========================================================================
+ */
+
+/*
+ * idt_csr_write() - CSR write operation
+ * @pdev: Pointer to the driver data
+ * @csraddr: CSR address (with no two LS bits)
+ * @data: Data to be written to CSR
+ */
+static int idt_csr_write(struct idt_89hpesx_dev *pdev, u16 csraddr,
+ const u32 data)
+{
+ struct device *dev = &pdev->client->dev;
+ struct idt_csr_seq csrseq;
+ struct idt_smb_seq smbseq;
+ int ret;
+
+ /* Initialize SMBus sequence fields */
+ smbseq.ccode = pdev->iniccode | CCODE_CSR;
+ smbseq.data = (u8 *)&csrseq;
+
+ /* Lock IDT SMBus device */
+ mutex_lock(&pdev->smb_mtx);
+
+ /* Perform write operation */
+ smbseq.bytecnt = CSR_WR_CNT;
+ csrseq.cmd = pdev->inicsrcmd | CSR_OP_WRITE;
+ csrseq.csraddr = cpu_to_le16(csraddr);
+ csrseq.data = cpu_to_le32(data);
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to write 0x%04x: 0x%04x to csr",
+ CSR_REAL_ADDR(csraddr), data);
+ goto err_mutex_unlock;
+ }
+
+ /* Send CSR address to read data from */
+ smbseq.bytecnt = CSR_WRRD_CNT;
+ csrseq.cmd = pdev->inicsrcmd | CSR_OP_READ;
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to init csr address 0x%04x",
+ CSR_REAL_ADDR(csraddr));
+ goto err_mutex_unlock;
+ }
+
+ /* Perform read operation */
+ smbseq.bytecnt = CSR_RD_CNT;
+ ret = pdev->smb_read(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to read csr 0x%04x",
+ CSR_REAL_ADDR(csraddr));
+ goto err_mutex_unlock;
+ }
+
+ /* Check whether IDT successfully retrieved CSR data */
+ if (csrseq.cmd & (CSR_RERR | CSR_WERR)) {
+ dev_err(dev, "IDT failed to perform CSR r/w");
+ ret = -EREMOTEIO;
+ goto err_mutex_unlock;
+ }
+
+ /* Unlock IDT SMBus device */
+err_mutex_unlock:
+ mutex_unlock(&pdev->smb_mtx);
+
+ return ret;
+}
+
+/*
+ * idt_csr_read() - CSR read operation
+ * @pdev: Pointer to the driver data
+ * @csraddr: CSR address (with no two LS bits)
+ * @data: Data to be written to CSR
+ */
+static int idt_csr_read(struct idt_89hpesx_dev *pdev, u16 csraddr, u32 *data)
+{
+ struct device *dev = &pdev->client->dev;
+ struct idt_csr_seq csrseq;
+ struct idt_smb_seq smbseq;
+ int ret;
+
+ /* Initialize SMBus sequence fields */
+ smbseq.ccode = pdev->iniccode | CCODE_CSR;
+ smbseq.data = (u8 *)&csrseq;
+
+ /* Lock IDT SMBus device */
+ mutex_lock(&pdev->smb_mtx);
+
+ /* Send CSR register address before reading it */
+ smbseq.bytecnt = CSR_WRRD_CNT;
+ csrseq.cmd = pdev->inicsrcmd | CSR_OP_READ;
+ csrseq.csraddr = cpu_to_le16(csraddr);
+ ret = pdev->smb_write(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to init csr address 0x%04x",
+ CSR_REAL_ADDR(csraddr));
+ goto err_mutex_unlock;
+ }
+
+ /* Perform read operation */
+ smbseq.bytecnt = CSR_RD_CNT;
+ ret = pdev->smb_read(pdev, &smbseq);
+ if (ret != 0) {
+ dev_err(dev, "Failed to read csr 0x%04hx",
+ CSR_REAL_ADDR(csraddr));
+ goto err_mutex_unlock;
+ }
+
+ /* Check whether IDT successfully retrieved CSR data */
+ if (csrseq.cmd & (CSR_RERR | CSR_WERR)) {
+ dev_err(dev, "IDT failed to perform CSR r/w");
+ ret = -EREMOTEIO;
+ goto err_mutex_unlock;
+ }
+
+ /* Save data retrieved from IDT */
+ *data = le32_to_cpu(csrseq.data);
+
+ /* Unlock IDT SMBus device */
+err_mutex_unlock:
+ mutex_unlock(&pdev->smb_mtx);
+
+ return ret;
+}
+
+/*===========================================================================
+ * Sysfs/debugfs-nodes IO-operations
+ *===========================================================================
+ */
+
+/*
+ * eeprom_write() - EEPROM sysfs-node write callback
+ * @filep: Pointer to the file system node
+ * @kobj: Pointer to the kernel object related to the sysfs-node
+ * @attr: Attributes of the file
+ * @buf: Buffer to write data to
+ * @off: Offset at which data should be written to
+ * @count: Number of bytes to write
+ */
+static ssize_t eeprom_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct idt_89hpesx_dev *pdev;
+ int ret;
+
+ /* Retrieve driver data */
+ pdev = dev_get_drvdata(kobj_to_dev(kobj));
+
+ /* Perform EEPROM write operation */
+ ret = idt_eeprom_write(pdev, (u16)off, (u16)count, (u8 *)buf);
+ return (ret != 0 ? ret : count);
+}
+
+/*
+ * eeprom_read() - EEPROM sysfs-node read callback
+ * @filep: Pointer to the file system node
+ * @kobj: Pointer to the kernel object related to the sysfs-node
+ * @attr: Attributes of the file
+ * @buf: Buffer to write data to
+ * @off: Offset at which data should be written to
+ * @count: Number of bytes to write
+ */
+static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct idt_89hpesx_dev *pdev;
+ int ret;
+
+ /* Retrieve driver data */
+ pdev = dev_get_drvdata(kobj_to_dev(kobj));
+
+ /* Perform EEPROM read operation */
+ ret = idt_eeprom_read(pdev, (u16)off, (u16)count, (u8 *)buf);
+ return (ret != 0 ? ret : count);
+}
+
+/*
+ * idt_dbgfs_csr_write() - CSR debugfs-node write callback
+ * @filep: Pointer to the file system file descriptor
+ * @buf: Buffer to read data from
+ * @count: Size of the buffer
+ * @offp: Offset within the file
+ *
+ * It accepts either "0x<reg addr>:0x<value>" for saving register address
+ * and writing value to specified DWORD register or "0x<reg addr>" for
+ * just saving register address in order to perform next read operation.
+ *
+ * WARNING No spaces are allowed. Incoming string must be strictly formated as:
+ * "<reg addr>:<value>". Register address must be aligned within 4 bytes
+ * (one DWORD).
+ */
+static ssize_t idt_dbgfs_csr_write(struct file *filep, const char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct idt_89hpesx_dev *pdev = filep->private_data;
+ char *colon_ch, *csraddr_str, *csrval_str;
+ int ret, csraddr_len, csrval_len;
+ u32 csraddr, csrval;
+ char *buf;
+
+ /* Copy data from User-space */
+ buf = kmalloc(count + 1, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = simple_write_to_buffer(buf, count, offp, ubuf, count);
+ if (ret < 0)
+ goto free_buf;
+ buf[count] = 0;
+
+ /* Find position of colon in the buffer */
+ colon_ch = strnchr(buf, count, ':');
+
+ /*
+ * If there is colon passed then new CSR value should be parsed as
+ * well, so allocate buffer for CSR address substring.
+ * If no colon is found, then string must have just one number with
+ * no new CSR value
+ */
+ if (colon_ch != NULL) {
+ csraddr_len = colon_ch - buf;
+ csraddr_str =
+ kmalloc(sizeof(char)*(csraddr_len + 1), GFP_KERNEL);
+ if (csraddr_str == NULL)
+ return -ENOMEM;
+ /* Copy the register address to the substring buffer */
+ strncpy(csraddr_str, buf, csraddr_len);
+ csraddr_str[csraddr_len] = '\0';
+ /* Register value must follow the colon */
+ csrval_str = colon_ch + 1;
+ csrval_len = strnlen(csrval_str, count - csraddr_len);
+ } else /* if (str_colon == NULL) */ {
+ csraddr_str = (char *)buf; /* Just to shut warning up */
+ csraddr_len = strnlen(csraddr_str, count);
+ csrval_str = NULL;
+ csrval_len = 0;
+ }
+
+ /* Convert CSR address to u32 value */
+ ret = kstrtou32(csraddr_str, 0, &csraddr);
+ if (ret != 0)
+ goto free_csraddr_str;
+
+ /* Check whether passed register address is valid */
+ if (csraddr > CSR_MAX || !IS_ALIGNED(csraddr, SZ_4)) {
+ ret = -EINVAL;
+ goto free_csraddr_str;
+ }
+
+ /* Shift register address to the right so to have u16 address */
+ pdev->csr = (csraddr >> 2);
+
+ /* Parse new CSR value and send it to IDT, if colon has been found */
+ if (colon_ch != NULL) {
+ ret = kstrtou32(csrval_str, 0, &csrval);
+ if (ret != 0)
+ goto free_csraddr_str;
+
+ ret = idt_csr_write(pdev, pdev->csr, csrval);
+ if (ret != 0)
+ goto free_csraddr_str;
+ }
+
+ /* Free memory only if colon has been found */
+free_csraddr_str:
+ if (colon_ch != NULL)
+ kfree(csraddr_str);
+
+ /* Free buffer allocated for data retrieved from User-space */
+free_buf:
+ kfree(buf);
+
+ return (ret != 0 ? ret : count);
+}
+
+/*
+ * idt_dbgfs_csr_read() - CSR debugfs-node read callback
+ * @filep: Pointer to the file system file descriptor
+ * @buf: Buffer to write data to
+ * @count: Size of the buffer
+ * @offp: Offset within the file
+ *
+ * It just prints the pair "0x<reg addr>:0x<value>" to passed buffer.
+ */
+#define CSRBUF_SIZE ((size_t)32)
+static ssize_t idt_dbgfs_csr_read(struct file *filep, char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct idt_89hpesx_dev *pdev = filep->private_data;
+ u32 csraddr, csrval;
+ char buf[CSRBUF_SIZE];
+ int ret, size;
+
+ /* Perform CSR read operation */
+ ret = idt_csr_read(pdev, pdev->csr, &csrval);
+ if (ret != 0)
+ return ret;
+
+ /* Shift register address to the left so to have real address */
+ csraddr = ((u32)pdev->csr << 2);
+
+ /* Print the "0x<reg addr>:0x<value>" to buffer */
+ size = snprintf(buf, CSRBUF_SIZE, "0x%05x:0x%08x\n",
+ (unsigned int)csraddr, (unsigned int)csrval);
+
+ /* Copy data to User-space */
+ return simple_read_from_buffer(ubuf, count, offp, buf, size);
+}
+
+/*
+ * eeprom_attribute - EEPROM sysfs-node attributes
+ *
+ * NOTE Size will be changed in compliance with OF node. EEPROM attribute will
+ * be read-only as well if the corresponding flag is specified in OF node.
+ */
+static BIN_ATTR_RW(eeprom, EEPROM_DEF_SIZE);
+
+/*
+ * csr_dbgfs_ops - CSR debugfs-node read/write operations
+ */
+static const struct file_operations csr_dbgfs_ops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = idt_dbgfs_csr_write,
+ .read = idt_dbgfs_csr_read
+};
+
+/*===========================================================================
+ * Driver init/deinit methods
+ *===========================================================================
+ */
+
+/*
+ * idt_set_defval() - disable EEPROM access by default
+ * @pdev: Pointer to the driver data
+ */
+static void idt_set_defval(struct idt_89hpesx_dev *pdev)
+{
+ /* If OF info is missing then use next values */
+ pdev->eesize = 0;
+ pdev->eero = true;
+ pdev->inieecmd = 0;
+ pdev->eeaddr = 0;
+}
+
+#ifdef CONFIG_OF
+static const struct i2c_device_id ee_ids[];
+/*
+ * idt_ee_match_id() - check whether the node belongs to compatible EEPROMs
+ */
+static const struct i2c_device_id *idt_ee_match_id(struct device_node *node)
+{
+ const struct i2c_device_id *id = ee_ids;
+ char devname[I2C_NAME_SIZE];
+
+ /* Retrieve the device name without manufacturer name */
+ if (of_modalias_node(node, devname, sizeof(devname)))
+ return NULL;
+
+ /* Search through the device name */
+ while (id->name[0]) {
+ if (strcmp(devname, id->name) == 0)
+ return id;
+ id++;
+ }
+ return NULL;
+}
+
+/*
+ * idt_get_ofdata() - get IDT i2c-device parameters from device tree
+ * @pdev: Pointer to the driver data
+ */
+static void idt_get_ofdata(struct idt_89hpesx_dev *pdev)
+{
+ const struct device_node *node = pdev->client->dev.of_node;
+ struct device *dev = &pdev->client->dev;
+
+ /* Read dts node parameters */
+ if (node) {
+ const struct i2c_device_id *ee_id = NULL;
+ struct device_node *child;
+ const __be32 *addr_be;
+ int len;
+
+ /* Walk through all child nodes looking for compatible one */
+ for_each_available_child_of_node(node, child) {
+ ee_id = idt_ee_match_id(child);
+ if (IS_ERR_OR_NULL(ee_id)) {
+ dev_warn(dev, "Skip unsupported child node %s",
+ child->full_name);
+ continue;
+ } else
+ break;
+ }
+
+ /* If there is no child EEPROM device, then set zero size */
+ if (!ee_id) {
+ idt_set_defval(pdev);
+ return;
+ }
+
+ /* Retrieve EEPROM size */
+ pdev->eesize = (u32)ee_id->driver_data;
+
+ /* Get custom EEPROM address from 'reg' attribute */
+ addr_be = of_get_property(child, "reg", &len);
+ if (!addr_be || (len < sizeof(*addr_be))) {
+ dev_warn(dev, "No reg on %s, use default address %d",
+ child->full_name, EEPROM_DEF_ADDR);
+ pdev->inieecmd = 0;
+ pdev->eeaddr = EEPROM_DEF_ADDR << 1;
+ } else {
+ pdev->inieecmd = EEPROM_USA;
+ pdev->eeaddr = be32_to_cpup(addr_be) << 1;
+ }
+
+ /* Check EEPROM 'read-only' flag */
+ if (of_get_property(child, "read-only", NULL))
+ pdev->eero = true;
+ else /* if (!of_get_property(node, "read-only", NULL)) */
+ pdev->eero = false;
+
+ dev_dbg(dev, "EEPROM of %u bytes found by %hhu",
+ pdev->eesize, pdev->eeaddr);
+ } else {
+ dev_warn(dev, "No dts node, EEPROM access disabled");
+ idt_set_defval(pdev);
+ }
+}
+#else
+static void idt_get_ofdata(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+
+ dev_warn(dev, "OF table is unsupported, EEPROM access disabled");
+
+ /* Nothing we can do, just set the default values */
+ idt_set_defval(pdev);
+}
+#endif /* CONFIG_OF */
+
+/*
+ * idt_create_pdev() - create and init data structure of the driver
+ * @client: i2c client of IDT PCIe-switch device
+ */
+static struct idt_89hpesx_dev *idt_create_pdev(struct i2c_client *client)
+{
+ struct idt_89hpesx_dev *pdev;
+
+ /* Allocate memory for driver data */
+ pdev = devm_kmalloc(&client->dev, sizeof(struct idt_89hpesx_dev),
+ GFP_KERNEL);
+ if (pdev == NULL)
+ return ERR_PTR(-ENOMEM);
+
+ /* Initialize basic fields of the data */
+ pdev->client = client;
+ i2c_set_clientdata(client, pdev);
+
+ /* Read OF nodes information */
+ idt_get_ofdata(pdev);
+
+ /* Initialize basic CSR CMD field - use full DWORD-sized r/w ops */
+ pdev->inicsrcmd = CSR_DWE;
+ pdev->csr = CSR_DEF;
+
+ /* Enable Packet Error Checking if it's supported by adapter */
+ if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_PEC)) {
+ pdev->iniccode = CCODE_PEC;
+ client->flags |= I2C_CLIENT_PEC;
+ } else /* PEC is unsupported */ {
+ pdev->iniccode = 0;
+ }
+
+ return pdev;
+}
+
+/*
+ * idt_free_pdev() - free data structure of the driver
+ * @pdev: Pointer to the driver data
+ */
+static void idt_free_pdev(struct idt_89hpesx_dev *pdev)
+{
+ /* Clear driver data from device private field */
+ i2c_set_clientdata(pdev->client, NULL);
+
+ /* Just free memory allocated for data */
+ devm_kfree(&pdev->client->dev, pdev);
+}
+
+/*
+ * idt_set_smbus_ops() - set supported SMBus operations
+ * @pdev: Pointer to the driver data
+ * Return status of smbus check operations
+ */
+static int idt_set_smbus_ops(struct idt_89hpesx_dev *pdev)
+{
+ struct i2c_adapter *adapter = pdev->client->adapter;
+ struct device *dev = &pdev->client->dev;
+
+ /* Check i2c adapter read functionality */
+ if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_BLOCK_DATA)) {
+ pdev->smb_read = idt_smb_read_block;
+ dev_dbg(dev, "SMBus block-read op chosen");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+ pdev->smb_read = idt_smb_read_i2c_block;
+ dev_dbg(dev, "SMBus i2c-block-read op chosen");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_WORD_DATA) &&
+ i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
+ pdev->smb_read = idt_smb_read_word;
+ dev_warn(dev, "Use slow word/byte SMBus read ops");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
+ pdev->smb_read = idt_smb_read_byte;
+ dev_warn(dev, "Use slow byte SMBus read op");
+ } else /* no supported smbus read operations */ {
+ dev_err(dev, "No supported SMBus read op");
+ return -EPFNOSUPPORT;
+ }
+
+ /* Check i2c adapter write functionality */
+ if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_BLOCK_DATA)) {
+ pdev->smb_write = idt_smb_write_block;
+ dev_dbg(dev, "SMBus block-write op chosen");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)) {
+ pdev->smb_write = idt_smb_write_i2c_block;
+ dev_dbg(dev, "SMBus i2c-block-write op chosen");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_WORD_DATA) &&
+ i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) {
+ pdev->smb_write = idt_smb_write_word;
+ dev_warn(dev, "Use slow word/byte SMBus write op");
+ } else if (i2c_check_functionality(adapter,
+ I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) {
+ pdev->smb_write = idt_smb_write_byte;
+ dev_warn(dev, "Use slow byte SMBus write op");
+ } else /* no supported smbus write operations */ {
+ dev_err(dev, "No supported SMBus write op");
+ return -EPFNOSUPPORT;
+ }
+
+ /* Initialize IDT SMBus slave interface mutex */
+ mutex_init(&pdev->smb_mtx);
+
+ return 0;
+}
+
+/*
+ * idt_check_dev() - check whether it's really IDT 89HPESx device
+ * @pdev: Pointer to the driver data
+ * Return status of i2c adapter check operation
+ */
+static int idt_check_dev(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+ u32 viddid;
+ int ret;
+
+ /* Read VID and DID directly from IDT memory space */
+ ret = idt_csr_read(pdev, IDT_VIDDID_CSR, &viddid);
+ if (ret != 0) {
+ dev_err(dev, "Failed to read VID/DID");
+ return ret;
+ }
+
+ /* Check whether it's IDT device */
+ if ((viddid & IDT_VID_MASK) != PCI_VENDOR_ID_IDT) {
+ dev_err(dev, "Got unsupported VID/DID: 0x%08x", viddid);
+ return -ENODEV;
+ }
+
+ dev_info(dev, "Found IDT 89HPES device VID:0x%04x, DID:0x%04x",
+ (viddid & IDT_VID_MASK), (viddid >> 16));
+
+ return 0;
+}
+
+/*
+ * idt_create_sysfs_files() - create sysfs attribute files
+ * @pdev: Pointer to the driver data
+ * Return status of operation
+ */
+static int idt_create_sysfs_files(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+ int ret;
+
+ /* Don't do anything if EEPROM isn't accessible */
+ if (pdev->eesize == 0) {
+ dev_dbg(dev, "Skip creating sysfs-files");
+ return 0;
+ }
+
+ /* Allocate memory for attribute file */
+ pdev->ee_file = devm_kmalloc(dev, sizeof(*pdev->ee_file), GFP_KERNEL);
+ if (!pdev->ee_file)
+ return -ENOMEM;
+
+ /* Copy the declared EEPROM attr structure to change some of fields */
+ memcpy(pdev->ee_file, &bin_attr_eeprom, sizeof(*pdev->ee_file));
+
+ /* In case of read-only EEPROM get rid of write ability */
+ if (pdev->eero) {
+ pdev->ee_file->attr.mode &= ~0200;
+ pdev->ee_file->write = NULL;
+ }
+ /* Create EEPROM sysfs file */
+ pdev->ee_file->size = pdev->eesize;
+ ret = sysfs_create_bin_file(&dev->kobj, pdev->ee_file);
+ if (ret != 0) {
+ kfree(pdev->ee_file);
+ dev_err(dev, "Failed to create EEPROM sysfs-node");
+ return ret;
+ }
+
+ return 0;
+}
+
+/*
+ * idt_remove_sysfs_files() - remove sysfs attribute files
+ * @pdev: Pointer to the driver data
+ */
+static void idt_remove_sysfs_files(struct idt_89hpesx_dev *pdev)
+{
+ struct device *dev = &pdev->client->dev;
+
+ /* Don't do anything if EEPROM wasn't accessible */
+ if (pdev->eesize == 0)
+ return;
+
+ /* Remove EEPROM sysfs file */
+ sysfs_remove_bin_file(&dev->kobj, pdev->ee_file);
+
+ /* Free memory allocated for bin_attribute structure */
+ kfree(pdev->ee_file);
+}
+
+/*
+ * idt_create_dbgfs_files() - create debugfs files
+ * @pdev: Pointer to the driver data
+ */
+#define CSRNAME_LEN ((size_t)32)
+static void idt_create_dbgfs_files(struct idt_89hpesx_dev *pdev)
+{
+ struct i2c_client *cli = pdev->client;
+ char fname[CSRNAME_LEN];
+
+ /* Create Debugfs directory for CSR file */
+ snprintf(fname, CSRNAME_LEN, "%d-%04hx", cli->adapter->nr, cli->addr);
+ pdev->csr_dir = debugfs_create_dir(fname, csr_dbgdir);
+
+ /* Create Debugfs file for CSR read/write operations */
+ pdev->csr_file = debugfs_create_file(cli->name, 0600,
+ pdev->csr_dir, pdev, &csr_dbgfs_ops);
+}
+
+/*
+ * idt_remove_dbgfs_files() - remove debugfs files
+ * @pdev: Pointer to the driver data
+ */
+static void idt_remove_dbgfs_files(struct idt_89hpesx_dev *pdev)
+{
+ /* Remove CSR directory and it sysfs-node */
+ debugfs_remove_recursive(pdev->csr_dir);
+}
+
+/*
+ * idt_probe() - IDT 89HPESx driver probe() callback method
+ */
+static int idt_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+ struct idt_89hpesx_dev *pdev;
+ int ret;
+
+ /* Create driver data */
+ pdev = idt_create_pdev(client);
+ if (IS_ERR(pdev))
+ return PTR_ERR(pdev);
+
+ /* Set SMBus operations */
+ ret = idt_set_smbus_ops(pdev);
+ if (ret != 0)
+ goto err_free_pdev;
+
+ /* Check whether it is truly IDT 89HPESx device */
+ ret = idt_check_dev(pdev);
+ if (ret != 0)
+ goto err_free_pdev;
+
+ /* Create sysfs files */
+ ret = idt_create_sysfs_files(pdev);
+ if (ret != 0)
+ goto err_free_pdev;
+
+ /* Create debugfs files */
+ idt_create_dbgfs_files(pdev);
+
+ return 0;
+
+err_free_pdev:
+ idt_free_pdev(pdev);
+
+ return ret;
+}
+
+/*
+ * idt_remove() - IDT 89HPESx driver remove() callback method
+ */
+static int idt_remove(struct i2c_client *client)
+{
+ struct idt_89hpesx_dev *pdev = i2c_get_clientdata(client);
+
+ /* Remove debugfs files first */
+ idt_remove_dbgfs_files(pdev);
+
+ /* Remove sysfs files */
+ idt_remove_sysfs_files(pdev);
+
+ /* Discard driver data structure */
+ idt_free_pdev(pdev);
+
+ return 0;
+}
+
+/*
+ * ee_ids - array of supported EEPROMs
+ */
+static const struct i2c_device_id ee_ids[] = {
+ { "24c32", 4096},
+ { "24c64", 8192},
+ { "24c128", 16384},
+ { "24c256", 32768},
+ { "24c512", 65536},
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, ee_ids);
+
+/*
+ * idt_ids - supported IDT 89HPESx devices
+ */
+static const struct i2c_device_id idt_ids[] = {
+ { "89hpes8nt2", 0 },
+ { "89hpes12nt3", 0 },
+
+ { "89hpes24nt6ag2", 0 },
+ { "89hpes32nt8ag2", 0 },
+ { "89hpes32nt8bg2", 0 },
+ { "89hpes12nt12g2", 0 },
+ { "89hpes16nt16g2", 0 },
+ { "89hpes24nt24g2", 0 },
+ { "89hpes32nt24ag2", 0 },
+ { "89hpes32nt24bg2", 0 },
+
+ { "89hpes12n3", 0 },
+ { "89hpes12n3a", 0 },
+ { "89hpes24n3", 0 },
+ { "89hpes24n3a", 0 },
+
+ { "89hpes32h8", 0 },
+ { "89hpes32h8g2", 0 },
+ { "89hpes48h12", 0 },
+ { "89hpes48h12g2", 0 },
+ { "89hpes48h12ag2", 0 },
+ { "89hpes16h16", 0 },
+ { "89hpes22h16", 0 },
+ { "89hpes22h16g2", 0 },
+ { "89hpes34h16", 0 },
+ { "89hpes34h16g2", 0 },
+ { "89hpes64h16", 0 },
+ { "89hpes64h16g2", 0 },
+ { "89hpes64h16ag2", 0 },
+
+ /* { "89hpes3t3", 0 }, // No SMBus-slave iface */
+ { "89hpes12t3g2", 0 },
+ { "89hpes24t3g2", 0 },
+ /* { "89hpes4t4", 0 }, // No SMBus-slave iface */
+ { "89hpes16t4", 0 },
+ { "89hpes4t4g2", 0 },
+ { "89hpes10t4g2", 0 },
+ { "89hpes16t4g2", 0 },
+ { "89hpes16t4ag2", 0 },
+ { "89hpes5t5", 0 },
+ { "89hpes6t5", 0 },
+ { "89hpes8t5", 0 },
+ { "89hpes8t5a", 0 },
+ { "89hpes24t6", 0 },
+ { "89hpes6t6g2", 0 },
+ { "89hpes24t6g2", 0 },
+ { "89hpes16t7", 0 },
+ { "89hpes32t8", 0 },
+ { "89hpes32t8g2", 0 },
+ { "89hpes48t12", 0 },
+ { "89hpes48t12g2", 0 },
+ { /* END OF LIST */ }
+};
+MODULE_DEVICE_TABLE(i2c, idt_ids);
+
+/*
+ * idt_driver - IDT 89HPESx driver structure
+ */
+static struct i2c_driver idt_driver = {
+ .driver = {
+ .name = IDT_NAME,
+ .owner = THIS_MODULE,
+ },
+ .probe = idt_probe,
+ .remove = idt_remove,
+ .id_table = idt_ids,
+};
+
+/*
+ * idt_init() - IDT 89HPESx driver init() callback method
+ */
+static int __init idt_init(void)
+{
+ /* Create Debugfs directory first */
+ if (debugfs_initialized())
+ csr_dbgdir = debugfs_create_dir("idt_csr", NULL);
+
+ /* Add new i2c-device driver */
+ return i2c_add_driver(&idt_driver);
+}
+module_init(idt_init);
+
+/*
+ * idt_exit() - IDT 89HPESx driver exit() callback method
+ */
+static void __exit idt_exit(void)
+{
+ /* Discard debugfs directory and all files if any */
+ debugfs_remove_recursive(csr_dbgdir);
+
+ /* Unregister i2c-device driver */
+ i2c_del_driver(&idt_driver);
+}
+module_exit(idt_exit);
--
2.6.6
^ permalink raw reply related
* [PATCH v5 2/2] eeprom: Add IDT 89HPESx driver bindings file
From: Serge Semin @ 2017-01-13 12:16 UTC (permalink / raw)
To: gregkh, srinivas.kandagatla, andrew, robh+dt, mark.rutland
Cc: Sergey.Semin, linux-kernel, devicetree, Serge Semin
In-Reply-To: <1484309813-25008-1-git-send-email-fancer.lancer@gmail.com>
IDT 89HPESx PCIe-switches exposes SMBus interface to have an access to
the device CSRs and EEPROM. So to properly utilize the interface
functionality, developer should declare a valid dts-file node, which
would refer to the corresponding 89HPESx device.
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
.../devicetree/bindings/misc/idt_89hpesx.txt | 44 ++++++++++++++++++++++
1 file changed, 44 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/idt_89hpesx.txt
diff --git a/Documentation/devicetree/bindings/misc/idt_89hpesx.txt b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
new file mode 100644
index 0000000..b9093b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/idt_89hpesx.txt
@@ -0,0 +1,44 @@
+EEPROM / CSR SMBus-slave interface of IDT 89HPESx devices
+
+Required properties:
+ - compatible : should be "<manufacturer>,<type>"
+ Basically there is only one manufacturer: idt, but some
+ compatible devices may be produced in future. Following devices
+ are supported: 89hpes8nt2, 89hpes12nt3, 89hpes24nt6ag2,
+ 89hpes32nt8ag2, 89hpes32nt8bg2, 89hpes12nt12g2, 89hpes16nt16g2,
+ 89hpes24nt24g2, 89hpes32nt24ag2, 89hpes32nt24bg2;
+ 89hpes12n3, 89hpes12n3a, 89hpes24n3, 89hpes24n3a;
+ 89hpes32h8, 89hpes32h8g2, 89hpes48h12, 89hpes48h12g2,
+ 89hpes48h12ag2, 89hpes16h16, 89hpes22h16, 89hpes22h16g2,
+ 89hpes34h16, 89hpes34h16g2, 89hpes64h16, 89hpes64h16g2,
+ 89hpes64h16ag2;
+ 89hpes12t3g2, 89hpes24t3g2, 89hpes16t4, 89hpes4t4g2,
+ 89hpes10t4g2, 89hpes16t4g2, 89hpes16t4ag2, 89hpes5t5,
+ 89hpes6t5, 89hpes8t5, 89hpes8t5a, 89hpes24t6, 89hpes6t6g2,
+ 89hpes24t6g2, 89hpes16t7, 89hpes32t8, 89hpes32t8g2,
+ 89hpes48t12, 89hpes48t12g2.
+ - reg : I2C address of the IDT 89HPESx device.
+
+Optionally there can be EEPROM-compatible subnode:
+ - compatible: There are five EEPROM devices supported: 24c32, 24c64, 24c128,
+ 24c256 and 24c512 differed by size.
+ - reg: Custom address of EEPROM device (If not specified IDT 89HPESx
+ (optional) device will try to communicate with EEPROM sited by default
+ address - 0x50)
+ - read-only : Parameterless property disables writes to the EEPROM
+ (optional)
+
+Example:
+ idt@60 {
+ compatible = "idt,89hpes32nt8ag2";
+ reg = <0x74>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ eeprom@50 {
+ compatible = "onsemi,24c64";
+ reg = <0x50>;
+ read-only;
+ };
+ };
+
--
2.6.6
^ permalink raw reply related
* Re: [PATCH v2 3/5] ARM: davinci_all_defconfig: enable iio and ADS7950
From: Sekhar Nori @ 2017-01-13 12:24 UTC (permalink / raw)
To: David Lechner
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Kevin Hilman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <b29c37b2-382c-dbab-966b-4463e3127676-l0cyMroinI0@public.gmane.org>
On Wednesday 11 January 2017 01:53 PM, Sekhar Nori wrote:
> On Tuesday 10 January 2017 09:13 PM, David Lechner wrote:
>> On 01/09/2017 06:29 AM, Sekhar Nori wrote:
>>> On Friday 06 January 2017 10:03 AM, David Lechner wrote:
>>>> This enables the iio subsystem and the TI ADS7950 driver. This is
>>>> used by
>>>> LEGO MINDSTORMS EV3, which has an ADS7957 chip.
>>>
>>> Can you add your sign-off?
>>>
>>>> ---
>>>>
>>>> The CONFIG_TI_ADS7950 driver is currently in iio/testing, so some
>>>> coordination
>>>> may be needed before picking up this patch.
>>>>
>>>> arch/arm/configs/davinci_all_defconfig | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/arch/arm/configs/davinci_all_defconfig
>>>> b/arch/arm/configs/davinci_all_defconfig
>>>> index 2b1967a..a899876 100644
>>>> --- a/arch/arm/configs/davinci_all_defconfig
>>>> +++ b/arch/arm/configs/davinci_all_defconfig
>>>> @@ -200,6 +200,13 @@ CONFIG_TI_EDMA=y
>>>> CONFIG_MEMORY=y
>>>> CONFIG_TI_AEMIF=m
>>>> CONFIG_DA8XX_DDRCTL=y
>>>> +CONFIG_IIO=m
>>>> +CONFIG_IIO_BUFFER_CB=m
>>>> +CONFIG_IIO_SW_DEVICE=m
>>>> +CONFIG_IIO_SW_TRIGGER=m
>>>
>>>> +CONFIG_TI_ADS7950=m
>>>
>>> Can you separate this from rest of the patch. I would like to enable
>>> this option only after I can find the symbol in linux-next.
>>
>> Will resend without CONFIG_TI_ADS7950
>>
>>>
>>>> +CONFIG_IIO_HRTIMER_TRIGGER=m
>>>> +CONFIG_IIO_SYSFS_TRIGGER=m
>>>
>>> Need CONFIG_IIO_TRIGGER=y also for these two options to take effect.
>>
>> CONFIG_IIO_TRIGGER is selected by IIO_TRIGGERED_BUFFER [=m] && IIO [=m]
>> && IIO_BUFFER [=y], so save_defconfig does not pick it up.
>
> I do remember I did not see these two modules did not get enabled in
> .config after 'make davinci_all_defconfig'. Will check what I may have
> missed.
So IIO_TRIGGERED_BUFFER is not selected in my tree because I dont have
the ADS7950 driver enabled. Same thing with IIO_BUFFER.
Can you try this patch over my tree?
Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox