Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 2/3] can: ti_hecc: Add TI HECC DT binding documentation
From: yegorslists @ 2017-01-11 14:05 UTC (permalink / raw)
  To: linux-can
  Cc: linux-omap, devicetree, robh+dt, jean-michel.hautbois,
	andrej.skvortzov, hs, Anton Glukhov, Yegor Yefremov
In-Reply-To: <1484143521-4898-1-git-send-email-yegorslists@googlemail.com>

From: Anton Glukhov <anton.a.glukhov@gmail.com>

DT binding documentation for TI High End CAN Controller

Signed-off-by: Anton Glukhov <anton.a.glukhov@gmail.com>
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
Changes v1 -> v2:
	change compatible to "ti,am3505"

 .../devicetree/bindings/net/can/ti_hecc.txt        | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/can/ti_hecc.txt

diff --git a/Documentation/devicetree/bindings/net/can/ti_hecc.txt b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
new file mode 100644
index 0000000..ce015cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/ti_hecc.txt
@@ -0,0 +1,31 @@
+* Texas Instruments High End CAN Controller (HECC)
+
+This file provides information, what the device node
+for the hecc interface contains.
+
+Required properties:
+- compatible: "ti,am3505"
+- reg: offset and length of the register set for the device
+- interrupts: interrupt mapping for the hecc interrupts sources
+- clocks: clock phandles (see clock bindings for details)
+- ti,scc-ram-offset: offset to scc module ram
+- ti,hecc-ram-offset: offset to hecc module ram
+- ti,mbx-offset: offset to mailbox ram
+
+Optional properties:
+- ti,int-line: interrupt line
+
+Example:
+
+For am3517evm board:
+	hecc: can@0x5c050000 {
+		compatible = "ti,am3505";
+		status = "disabled";
+		reg = <0x5c050000 0x4000>;
+		interrupts = <24>;
+		clocks = <&hecc_ck>;
+		ti,scc-ram-offset = <0x3000>;
+		ti,hecc-ram-offset = <0x3000>;
+		ti,mbx-offset = <0x2000>;
+		ti,int-line = <0>;
+	};
-- 
2.1.4


^ permalink raw reply related

* [PATCH v2 3/3] can: ti_hecc: Add DT support for TI HECC module
From: yegorslists-gM/Ye1E23mwN+BqQ9rBEUg @ 2017-01-11 14:05 UTC (permalink / raw)
  To: linux-can-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	jean-michel.hautbois-B+Q8N6RmIDZBDgjK7y7TUQ,
	andrej.skvortzov-Re5JQEeQqe8AvxtiuMwx3w, hs-ynQEQJNshbs,
	Anton Glukhov, Yegor Yefremov
In-Reply-To: <1484143521-4898-1-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

From: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

These patch set adds device tree support for TI HECC module.

Signed-off-by: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
Changes v1 -> v2:
	- change compatible to "ti,am3505"
	- remove CONFIG_OF
	- don't set int_line to 0 explicitly

 drivers/net/can/ti_hecc.c | 54 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 50 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index 680d1ff..fd3d9fc 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -46,6 +46,8 @@
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 
 #include <linux/can/dev.h>
 #include <linux/can/error.h>
@@ -872,19 +874,62 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
 	.ndo_change_mtu		= can_change_mtu,
 };
 
+static const struct of_device_id ti_hecc_dt_ids[] = {
+	{
+		.compatible = "ti,am3505",
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_hecc_dt_ids);
+
+static struct ti_hecc_platform_data *hecc_parse_dt(struct device *dev)
+{
+	struct ti_hecc_platform_data *pdata;
+	struct device_node *np = dev->of_node;
+
+	pdata = devm_kzalloc(dev, sizeof(struct ti_hecc_platform_data), GFP_KERNEL);
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	if (of_property_read_u32(np, "ti,scc-ram-offset", &pdata->scc_ram_offset)) {
+		dev_err(dev, "Missing scc-ram-offset property in the DT.\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32(np, "ti,hecc-ram-offset", &pdata->hecc_ram_offset)) {
+		dev_err(dev, "Missing hecc-ram-offset property in the DT.\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (of_property_read_u32(np, "ti,mbx-offset", &pdata->mbx_offset)) {
+		dev_err(dev, "Missing mbx-offset property in the DT.\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	of_property_read_u32(dev->of_node, "ti,int-line", &pdata->int_line);
+
+	return pdata;
+}
+
 static int ti_hecc_probe(struct platform_device *pdev)
 {
 	struct net_device *ndev = (struct net_device *)0;
 	struct ti_hecc_priv *priv;
-	struct ti_hecc_platform_data *pdata;
+	struct ti_hecc_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *mem, *irq;
 	void __iomem *addr;
 	int err = -ENODEV;
 
-	pdata = dev_get_platdata(&pdev->dev);
+	if (!pdata && np) {
+		pdata = hecc_parse_dt(&pdev->dev);
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	}
+
 	if (!pdata) {
-		dev_err(&pdev->dev, "No platform data\n");
-		goto probe_exit;
+		dev_err(&pdev->dev, "Platform data missing\n");
+		return -EINVAL;
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1037,6 +1082,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
 static struct platform_driver ti_hecc_driver = {
 	.driver = {
 		.name    = DRV_NAME,
+		.of_match_table = ti_hecc_dt_ids,
 	},
 	.probe = ti_hecc_probe,
 	.remove = ti_hecc_remove,
-- 
2.1.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 related

* Re: [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: M'boumba Cedric Madianga @ 2017-01-11 14:20 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Wolfram Sang, Rob Herring, Maxime Coquelin, Alexandre Torgue,
	Linus Walleij, Patrice Chotard, Russell King, linux-i2c,
	devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <CAOAejn2eOy2sn1VkE979ne23Sj9L6+kaQDNpL1EUKb2m=6sGXw@mail.gmail.com>

>
>> +              */
>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
>
> You could get rid of this, when caching the value of CR1. Would save two
> register reads here. This doesn't work for all registers, but it should
> be possible to apply for most of them, maybe enough to get rid of the
> clr_bits and set_bits function.

I agree at many places I could save registers read by not using
clr_bits and set_bits function when the registers in question has been
already read.
But it is not enough to get rid of the clr_bits and set_bits function.
For example when calling stm32f4_i2c_terminate_xfer(), the CR1
register is never read before so set_bits function is useful.
Another example, when stm32f4_i2c_handle_rx_done(), the CR1 register
is also never read before so clr_bits finction is again useful.

2017-01-11 14:58 GMT+01:00 M'boumba Cedric Madianga <cedric.madianga@gmail.com>:
> Hi Uwe,
>
> 2017-01-11 9:22 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
>> Hello Cedric,
>>
>> On Thu, Jan 05, 2017 at 10:07:23AM +0100, M'boumba Cedric Madianga wrote:
>>> +/*
>>> + * In standard mode:
>>> + * SCL period = SCL high period = SCL low period = CCR * I2C parent clk period
>>> + *
>>> + * In fast mode:
>>> + * If Duty = 0; SCL high period = 1  * CCR * I2C parent clk period
>>> + *           SCL low period  = 2  * CCR * I2C parent clk period
>>> + * If Duty = 1; SCL high period = 9  * CCR * I2C parent clk period
>>> + *           SCL low period  = 16 * CCR * I2C parent clk period
>> s/  \*/ */ several times
>
> Sorry but I don't see where is the issue as the style for multi-line
> comments seems ok.
> Could you please clarify that point if possible ? Thanks in advance
>
>>
>>> + * In order to reach 400 kHz with lower I2C parent clk frequencies we always set
>>> + * Duty = 1
>>> + *
>>> + * For both modes, we have CCR = SCL period * I2C parent clk frequency
>>> + * with scl_period = 5 microseconds in Standard mode and scl_period = 1
>> s/mode/Mode/
>
> ok thanks
>
>>
>>> + * microsecond in Fast Mode in order to satisfy scl_high and scl_low periods
>>> + * constraints defined by i2c bus specification
>>
>> I don't understand scl_period = 1 µs for Fast Mode. For a bus freqency
>> of 400 kHz we need low + high = 2.5 µs. Is there a factor 10 missing
>> somewhere?
>
> As CCR = SCL_period * I2C parent clk frequency with minimal freq =
> 2Mhz and SCL_period = 1 we have:
> CCR = 1 * 2Mhz = 2.
> But to compute, scl_low and scl_high in Fast mode, we have to do the
> following thing as Duty=1:
> scl_high = 9 * CCR * I2C parent clk period
> scl_low = 16 * CCR * I2C parent clk period
> In our example:
> scl_high = 9 * 2 * 0,0000005 = 0,000009 sec = 9 µs
> scl_low = 16 * 2 * 0.0000005 = 0,000016 sec = 16 µs
> So low + high = 27 µs > 2,5 µs
>
>>
>>> + */
>>> +static struct stm32f4_i2c_timings i2c_timings[] = {
>>> [...]
>>> +
>>> +/**
>>> + * stm32f4_i2c_hw_config() - Prepare I2C block
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static int stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> +     int ret = 0;
>>> +
>>> +     /* Disable I2C */
>>> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
>>> +
>>> +     ret = stm32f4_i2c_set_periph_clk_freq(i2c_dev);
>>> +     if (ret)
>>> +             return ret;
>>> +
>>> +     stm32f4_i2c_set_rise_time(i2c_dev);
>>> +
>>> +     stm32f4_i2c_set_speed_mode(i2c_dev);
>>> +
>>> +     stm32f4_i2c_set_filter(i2c_dev);
>>> +
>>> +     /* Enable I2C */
>>> +     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
>>
>> This function is called after a hw reset, so there should be no need to
>> use clr_bits and set_bits because the value read from hw should be
>> known.
>
> ok thanks
>
>>
>>> +     return ret;
>>
>> return 0;
>
> ok thanks
>
>>
>>> +}
>>> +
>>> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> +     u32 status;
>>> +     int ret;
>>> +
>>> +     ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
>>> +                                      status,
>>> +                                      !(status & STM32F4_I2C_SR2_BUSY),
>>> +                                      10, 1000);
>>> +     if (ret) {
>>> +             dev_dbg(i2c_dev->dev, "bus not free\n");
>>> +             ret = -EBUSY;
>>> +     }
>>> +
>>> +     return ret;
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_write_ byte() - Write a byte in the data register
>>> + * @i2c_dev: Controller's private data
>>> + * @byte: Data to write in the register
>>> + */
>>> +static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
>>> +{
>>> +     writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_write_msg() - Fill the data register in write mode
>>> + * @i2c_dev: Controller's private data
>>> + *
>>> + * This function fills the data register with I2C transfer buffer
>>> + */
>>> +static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> +
>>> +     stm32f4_i2c_write_byte(i2c_dev, *msg->buf++);
>>> +     msg->count--;
>>> +}
>>> +
>>> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> +     u32 rbuf;
>>> +
>>> +     rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
>>> +     *msg->buf++ = rbuf & 0xff;
>>
>> This is unnecessary. buf has an 8 bit wide type so
>>
>>         *msg->buf++ = rbuf;
>>
>> has the same effect. (ISTR this is something I already pointed out
>> earlier?)
>
> Yes you are right.
>
>>
>>> +     msg->count--;
>>> +}
>>> +
>>> +static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>>> +
>>> +     stm32f4_i2c_disable_irq(i2c_dev);
>>> +
>>> +     reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> +     if (msg->stop)
>>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>>> +     else
>>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>>> +
>>> +     complete(&i2c_dev->complete);
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>>> +
>>> +     if (msg->count) {
>>> +             stm32f4_i2c_write_msg(i2c_dev);
>>> +             if (!msg->count) {
>>> +                     /* Disable buffer interrupts for RXNE/TXE events */
>>> +                     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>>> +             }
>>> +     } else {
>>> +             stm32f4_i2c_terminate_xfer(i2c_dev);
>>
>> Is stm32f4_i2c_terminate_xfer also called when arbitration is lost? If
>> yes, is it then right to set STM32F4_I2C_CR1_STOP or
>> STM32F4_I2C_CR1_START?
>
> If arbitration is lost, stm32f4_i2c_terminate_xfer() is not called.
> In that case, we return -EAGAIN and i2c-core will retry by calling
> stm32f4_i2c_xfer()
>
>>
>>> +     }
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
>>> +
>>> +     switch (msg->count) {
>>> +     case 1:
>>> +             stm32f4_i2c_disable_irq(i2c_dev);
>>> +             stm32f4_i2c_read_msg(i2c_dev);
>>> +             complete(&i2c_dev->complete);
>>> +             break;
>>> +     /*
>>> +      * For 2 or 3-byte reception, we do not have to read the data register
>>> +      * when RXNE occurs as we have to wait for byte transferred finished
>>
>> it's hard to understand because if you don't know the hardware the
>> meaning of RXNE is unknown.
>
> Ok I will replace RXNE by RX not empty in that comment
>
>>
>>> +      * event before reading data. So, here we just disable buffer
>>> +      * interrupt in order to avoid another system preemption due to RXNE
>>> +      * event
>>> +      */
>>> +     case 2:
>>> +     case 3:
>>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
>>> +             break;
>>> +     /* For N byte reception with N > 3 we directly read data register */
>>> +     default:
>>> +             stm32f4_i2c_read_msg(i2c_dev);
>>> +     }
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
>>> + * in case of read
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>
>> btf is a hw-related name. Maybe better use _done which is easier to
>> understand?
>
> OK
>
>>
>>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> +     void __iomem *reg;
>>> +     u32 mask;
>>> +     int i;
>>> +
>>> +     switch (msg->count) {
>>> +     case 2:
>>> +             /*
>>> +              * In order to correctly send the Stop or Repeated Start
>>> +              * condition on the I2C bus, the STOP/START bit has to be set
>>> +              * before reading the last two bytes.
>>> +              * After that, we could read the last two bytes, disable
>>> +              * remaining interrupts and notify the end of xfer to the
>>> +              * client
>>
>> This is surprising. I didn't recheck the manual, but that looks very
>> uncomfortable.
>
> I agree but this exactly the hardware way of working described in the
> reference manual.
>
>>How does this work, when I only want to read a single
>> byte? Same problem for ACK below.
>
> For a single reception, we enable NACK and STOP or Repeatead START
> bits during address match.
> The NACK and STOP/START pulses are sent as soon as the data is
> received in the shift register.
> Please note that in that case, we don't have to wait BTF event to read the data.
> Data is read as soon as RXNE event occurs.
>
>>
>>> +              */
>>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> +             if (msg->stop)
>>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>>> +             else
>>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>>> +
>>> +             for (i = 2; i > 0; i--)
>>> +                     stm32f4_i2c_read_msg(i2c_dev);
>>> +
>>> +             reg = i2c_dev->base + STM32F4_I2C_CR2;
>>> +             mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
>>> +             stm32f4_i2c_clr_bits(reg, mask);
>>> +
>>> +             complete(&i2c_dev->complete);
>>> +             break;
>>> +     case 3:
>>> +             /*
>>> +              * In order to correctly send the ACK on the I2C bus for the
>>> +              * last two bytes, we have to set ACK bit before reading the
>>> +              * third last data byte
>>> +              */
>>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>>> +             stm32f4_i2c_read_msg(i2c_dev);
>>> +             break;
>>> +     default:
>>> +             stm32f4_i2c_read_msg(i2c_dev);
>>> +     }
>>> +}
>>> +
>>> +/**
>>> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
>>> + * master receiver
>>> + * @i2c_dev: Controller's private data
>>> + */
>>> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
>>> +{
>>> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
>>> +     void __iomem *reg;
>>> +
>>> +     switch (msg->count) {
>>> +     case 0:
>>> +             stm32f4_i2c_terminate_xfer(i2c_dev);
>>> +             /* Clear ADDR flag */
>>> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>>> +             break;
>>> +     case 1:
>>> +             /*
>>> +              * Single byte reception:
>>
>> This also happens for the last byte of a 5 byte transfer, right?
>
> For a 5 byte transfer the behavior is different:
> We have to read data from DR (data register)  as soon as the RXNE (RX
> not empty) event occurs for data1, data2 and data3 (until N-2 data for
> a more generic case)
> The ACK is automatically sent as soon as the data is received in the
> shift register as the I2C controller was configured to do that during
> adress match phase.
>
> For data3 (N-2 data), we wait for BTF (Byte Transfer finished) event
> in order to set NACK before reading DR.
> This event occurs when a new data has been received in shift register
> (in our case data4 or N-1 data) but the prevoius data in DR (in our
> case data3 or N-2 data) has not been read yet.
> In that way, the NACK pulse will be correctly generated after the last
> received data byte.
>
> For data4 and data5, we wait for BTF event (data4 or N-1 data in DR
> and data5 or N data in shift register), set STOP or repeated Start in
> order to correctly sent the right pulse after the last received data
> byte and run 2 consecutives read of DR.
>
>>
>>> +              * Enable NACK, clear ADDR flag and generate STOP or RepSTART
>>> +              */
>>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>>> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>>> +             if (msg->stop)
>>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
>>> +             else
>>> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
>>> +             break;
>>> +     case 2:
>>> +             /*
>>> +              * 2-byte reception:
>>> +              * Enable NACK and set POS
>>
>> What is POS?
> POS is used to define the position of the (N)ACK pulse
> 0: ACK is generated when the current is being received in the shift register
> 1: ACK is generated when the next byte which will be received in the
> shift register (used for 2-byte reception)
>
>>
>>> +              */
>>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
>>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
>>
>> You could get rid of this, when caching the value of CR1. Would save two
>> register reads here. This doesn't work for all registers, but it should
>> be possible to apply for most of them, maybe enough to get rid of the
>> clr_bits and set_bits function.
>>
>>> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
>>> +             break;
>>> +
>>> +     default:
>>> +             /* N-byte reception: Enable ACK */
>>> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
>>> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
>>
>> Do you need to set ACK for each byte transferred?
> I need to do that in order to be SMBus compatible and the ACK/NACK
> seems to be used by default in Documentation/i2c/i2c-protocol file.
>
>>
>> I stopp reviewing here because of -ENOTIME on my side but don't want to
>> delay discussion, so sent my comments up to here already now.
>>
>> Best regards
>> Uwe
>>
>> --
>> Pengutronix e.K.                           | Uwe Kleine-König            |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH 0/3] arm64: dts: A64 board MMC support
From: Andre Przywara @ 2017-01-11 14:24 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, Rob Herring, Mark Rutland, linux-arm-kernel,
	devicetree, linux-sunxi, linux-kernel
In-Reply-To: <20170111135139.ktirdwmzhyv3cn2q@lukather>

Hi,

On 11/01/17 13:51, Maxime Ripard wrote:
> Hi,
> 
> On Tue, Jan 10, 2017 at 01:22:30AM +0000, Andre Przywara wrote:
>> These patches here go on top of Maxime's latest A64 MMC series and
>> enable the MMC controllers on the boards using the A64 SoC.
>> As the BananaPi-M64 DT now looks different, lets adds support for
>> that board as well, with one major difference to the Pine64 being the eMMC
>> chip.
>>
>> I could't find commit f9ca9b952ee1 Maxime mentioned in his cover letter,
>> so I applied his patches on top of sunxi/for-next and cherry-picked
>> b4b8664d29 to fix the arm64 build.
> 
> I'm fine with all your patches. I've queued them in my MMC series, and
> will continue pushing them and / or merging them if it's ok for you.

Yes, please do!

Many thanks!

Cheers,
Andre.

^ permalink raw reply

* Re: [PATCH v2 3/3] can: ti_hecc: Add DT support for TI HECC module
From: Yegor Yefremov @ 2017-01-11 14:24 UTC (permalink / raw)
  To: linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	Andrey Skvortsov, hs-ynQEQJNshbs, Anton Glukhov, Yegor Yefremov
In-Reply-To: <1484143521-4898-4-git-send-email-yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

On Wed, Jan 11, 2017 at 3:05 PM,  <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org> wrote:
> From: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> These patch set adds device tree support for TI HECC module.
>
> Signed-off-by: Anton Glukhov <anton.a.glukhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Yegor Yefremov <yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> ---
> Changes v1 -> v2:
>         - change compatible to "ti,am3505"
>         - remove CONFIG_OF
>         - don't set int_line to 0 explicitly
>
>  drivers/net/can/ti_hecc.c | 54 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
> index 680d1ff..fd3d9fc 100644
> --- a/drivers/net/can/ti_hecc.c
> +++ b/drivers/net/can/ti_hecc.c
> @@ -46,6 +46,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>
>  #include <linux/can/dev.h>
>  #include <linux/can/error.h>
> @@ -872,19 +874,62 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
>         .ndo_change_mtu         = can_change_mtu,
>  };
>
> +static const struct of_device_id ti_hecc_dt_ids[] = {
> +       {
> +               .compatible = "ti,am3505",
> +       },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, ti_hecc_dt_ids);
> +
> +static struct ti_hecc_platform_data *hecc_parse_dt(struct device *dev)
> +{
> +       struct ti_hecc_platform_data *pdata;
> +       struct device_node *np = dev->of_node;
> +
> +       pdata = devm_kzalloc(dev, sizeof(struct ti_hecc_platform_data), GFP_KERNEL);
> +       if (!pdata)
> +               return ERR_PTR(-ENOMEM);
> +
> +       if (of_property_read_u32(np, "ti,scc-ram-offset", &pdata->scc_ram_offset)) {
> +               dev_err(dev, "Missing scc-ram-offset property in the DT.\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (of_property_read_u32(np, "ti,hecc-ram-offset", &pdata->hecc_ram_offset)) {
> +               dev_err(dev, "Missing hecc-ram-offset property in the DT.\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       if (of_property_read_u32(np, "ti,mbx-offset", &pdata->mbx_offset)) {
> +               dev_err(dev, "Missing mbx-offset property in the DT.\n");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       of_property_read_u32(dev->of_node, "ti,int-line", &pdata->int_line);
> +
> +       return pdata;
> +}
> +
>  static int ti_hecc_probe(struct platform_device *pdev)
>  {
>         struct net_device *ndev = (struct net_device *)0;
>         struct ti_hecc_priv *priv;
> -       struct ti_hecc_platform_data *pdata;
> +       struct ti_hecc_platform_data *pdata = dev_get_platdata(&pdev->dev);
> +       struct device_node *np = pdev->dev.of_node;
>         struct resource *mem, *irq;
>         void __iomem *addr;
>         int err = -ENODEV;
>
> -       pdata = dev_get_platdata(&pdev->dev);
> +       if (!pdata && np) {
> +               pdata = hecc_parse_dt(&pdev->dev);
> +               if (IS_ERR(pdata))
> +                       return PTR_ERR(pdata);
> +       }
> +
>         if (!pdata) {
> -               dev_err(&pdev->dev, "No platform data\n");
> -               goto probe_exit;
> +               dev_err(&pdev->dev, "Platform data missing\n");
> +               return -EINVAL;
>         }
>
>         mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -1037,6 +1082,7 @@ static int ti_hecc_resume(struct platform_device *pdev)
>  static struct platform_driver ti_hecc_driver = {
>         .driver = {
>                 .name    = DRV_NAME,
> +               .of_match_table = ti_hecc_dt_ids,
>         },
>         .probe = ti_hecc_probe,
>         .remove = ti_hecc_remove,
> --
> 2.1.4
>

So far the CAN interface will be registered, but I get following issue
with clock for HECC:

[    6.574174] CAN device driver interface
[    6.663318] ------------[ cut here ]------------
[    6.668220] WARNING: CPU: 0 PID: 115 at drivers/clk/clk.c:652
clk_core_enable+0xdc/0x268
[    6.676676] Modules linked in: ti_hecc(+) can_dev
snd_soc_omap_mcbsp omap_wdt snd_soc_omap snd_soc_tlv320aic23_i2c
snd_soc_tlv320aic23 snd_soc_core ehci_omap snd_pcm_dmaengine snd_pcm
ehci_hcd snd_timer snd at24 soundcore nvmem_core gpio_pca953x usbcore
[    6.700227] CPU: 0 PID: 115 Comm: udevd Not tainted 4.10.0-rc3 #2
[    6.706595] Hardware name: Generic AM3517 (Flattened Device Tree)
[    6.713000] [<c010fff8>] (unwind_backtrace) from [<c010c178>]
(show_stack+0x10/0x14)
[    6.721108] [<c010c178>] (show_stack) from [<c0518fcc>]
(dump_stack+0xac/0xe0)
[    6.728682] [<c0518fcc>] (dump_stack) from [<c0136d60>] (__warn+0xd8/0x104)
[    6.735973] [<c0136d60>] (__warn) from [<c0136e38>]
(warn_slowpath_null+0x20/0x28)
[    6.743898] [<c0136e38>] (warn_slowpath_null) from [<c0595530>]
(clk_core_enable+0xdc/0x268)
[    6.752730] [<c0595530>] (clk_core_enable) from [<c05966d0>]
(clk_core_enable_lock+0x18/0x2c)
[    6.761670] [<c05966d0>] (clk_core_enable_lock) from [<bf17202c>]
(ti_hecc_probe+0x188/0x3d0 [ti_hecc])
[    6.771616] [<bf17202c>] (ti_hecc_probe [ti_hecc]) from
[<c05f5930>] (platform_drv_probe+0x50/0xb0)
[    6.781095] [<c05f5930>] (platform_drv_probe) from [<c05f3998>]
(driver_probe_device+0x1f8/0x2cc)
[    6.790379] [<c05f3998>] (driver_probe_device) from [<c05f3b2c>]
(__driver_attach+0xc0/0xc4)
[    6.799208] [<c05f3b2c>] (__driver_attach) from [<c05f1e3c>]
(bus_for_each_dev+0x6c/0xa0)
[    6.807763] [<c05f1e3c>] (bus_for_each_dev) from [<c05f2ef8>]
(bus_add_driver+0x100/0x210)
[    6.816410] [<c05f2ef8>] (bus_add_driver) from [<c05f4968>]
(driver_register+0x78/0xf4)
[    6.824785] [<c05f4968>] (driver_register) from [<c0101874>]
(do_one_initcall+0x3c/0x170)
[    6.833342] [<c0101874>] (do_one_initcall) from [<c023b4c8>]
(do_init_module+0x5c/0x1b8)
[    6.841818] [<c023b4c8>] (do_init_module) from [<c01d9a08>]
(load_module+0x1d2c/0x23f8)
[    6.850194] [<c01d9a08>] (load_module) from [<c01da2f0>]
(SyS_finit_module+0xa4/0xb8)
[    6.858392] [<c01da2f0>] (SyS_finit_module) from [<c0107760>]
(ret_fast_syscall+0x0/0x1c)
[    6.866939] ---[ end trace b502c707901327dc ]---
[    6.875799] ti_hecc 5c050000.can: device registered
(reg_base=d0b1c000, irq=40)

Going to take a closer look at this.

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

^ permalink raw reply

* Re: [PATCH v2 05/12] Document: dt: binding: imx: update pinctrl doc for imx6sll
From: Linus Walleij @ 2017-01-11 14:33 UTC (permalink / raw)
  To: Jacky Bai
  Cc: Shawn Guo, Michael Turquette, Stephen Boyd, Rob Herring,
	Mark Rutland, Sascha Hauer, Fabio Estevam, Daniel Lezcano,
	Thomas Gleixner, Philipp Zabel, linux-clk,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	jacky.baip-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
In-Reply-To: <AM3PR04MB5304992095753B761E66A9A87640-f56W/S9L6NRDGcFijYBXZgfhPeD8jYilXA4E9RH9d+qIuWR1G4zioA@public.gmane.org>

On Mon, Jan 9, 2017 at 3:32 AM, Jacky Bai <ping.bai-3arQi8VN3Tc@public.gmane.org> wrote:

> I have look into the above commit on using generic binding. But I think the generic pinconf
> is not very easy to add in imx pinctrl Driver. imx pinctrl use a different way to parse the pin configure.

OK atleast I need an indication from one of the i.MX maintainers how they
want to proceed.

> Each fsl,pin entry  looks like <PIN_FUNC_ID  CONFIG> in dts, the CONFIG is the pad setting value like
> pull-up, open-drain, drive strength etc. The above config bit definition is specific to each SOC in the PAD CTL register.
>
> If we want set the pin config to enable hysteresis, 47KOhm Pull Up, 50Mhz speed, 80Ohm driver strength
> and Fast Slew Rate, then the CONFIG value should be 0x17059( ORs corresponding bit definition).

Hysteresis is an input property and does not make sense on something
that need drive
strength and slew rate configuration which are output properties.
(I guess you mean 80mA drive strength.)

Actually such oxymoronic settings is a good reason to migrate to
generic bindings
because when you describe stuff with generic strings you see better
what is going
on, and we can add sanity checks to cases like this in the generic code where it
would indeed be valid to ask why this combination of settings is being made.

> This value will be set in
> PAD CTL register to config the corresponding pin.

Yes? That is common. It looks like that in DT:

{
   input-schmitt-enable;
   bias-pull-up = <47000>;
   slew-rate = <50000000>;
   drive-strength = <80000>;
};

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

^ permalink raw reply

* Re: [PATCH v2 9/9] ARM: sunxi: Convert pinctrl nodes to generic bindings
From: Linus Walleij @ 2017-01-11 14:41 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: André Przywara, Chen-Yu Tsai, devicetree, linux-kernel,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel
In-Reply-To: <20170109111630.pcsjmc5l4v7vi2rm@lukather>

On Mon, Jan 9, 2017 at 12:16 PM, Maxime Ripard
<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Fri, Jan 06, 2017 at 01:17:21AM +0000, André Przywara wrote:
>> > On Wed, Jan 04, 2017 at 02:16:23AM +0000, André Przywara wrote:
>> >> So can I ask that we start taking this seriously and stop doing things
>> >> which prevent Allwinner boards from being supported properly?
>> >> Which would first involve dropping this very patch?
>> >
>> > The driver still supports the old binding.
>>
>> Yes, a _current_ version of the driver supports both bindings, but older
>> versions *require* the older binding and bail out if various
>> allwinner,xxx properties are missing - as in those proposed new DTs:
>>
>> 4.9 kernel with sunxi/for-next .dtb:
>> sun8i-h3-pinctrl 1c20800.pinctrl: missing allwinner,function property in
>> node uart0
>> sun8i-h3-pinctrl 1c20800.pinctrl: missing allwinner,function property in
>> node mmc0
>> sunxi-mmc: probe of 1c0f000.mmc failed with error -22
>
> This is seriously getting out of control. We already come to great
> length (and sometimes a painful amount of hacks) to satisfy a few
> individuals with a theorical interest in backward compatibility (and
> apparently, we're even the only one doing so, even more platforms
> choosing to not support that as we speak), there's seriously no reason
> to support forward compatibility as well. This has *never* been a
> thing, never has been documented nor advertised, I don't know why it
> should be one more thing to carry on our shoulders.

I agree.

We have too much standardization burden to maintain already as it is.

And AFAICT the Allwinner support is a hacker/community/enthusiast
effort without vendor backing.

We should be aware that the strong push toward DT standardization
was due to mess in the vendor trees, and as for the ambition to ship
DTBs with products, that is for people producing products to do,
not for the community. Community support can very well use attached
DTB files on the kernel from my point of view, I don't see why
ArchLinuxARM and others should have to standardize and support
DTB file in flash images, very ambitious if they do but definately
not their problem.

Yours,
Linus Walleij
--
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: [PATCHv3 3/5] pinctrl: mvebu: pinctrl driver for 98DX3236 SoC
From: Linus Walleij @ 2017-01-11 14:44 UTC (permalink / raw)
  To: Chris Packham, Gregory CLEMENT, Thomas Petazzoni,
	Sebastian Hesselbarth
  Cc: linux-arm-kernel@lists.infradead.org, Kalyan Kinthada,
	Rob Herring, Mark Rutland, Laxman Dewangan,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20170106041517.9589-4-chris.packham@alliedtelesis.co.nz>

On Fri, Jan 6, 2017 at 5:15 AM, Chris Packham
<chris.packham@alliedtelesis.co.nz> 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>

I am waiting for an ACK or comment from the maintainers on
this patch. Sebastian?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v1 1/1] iio: adc: tlc4541: add support for TI tlc4541 adc
From: Jonathan Cameron @ 2017-01-11 14:47 UTC (permalink / raw)
  To: Peter Meerwald-Stadler, Phil Reid
  Cc: jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <alpine.DEB.2.02.1701111007240.12204-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>



On 11 January 2017 09:17:11 GMT+00:00, Peter Meerwald-Stadler <pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org> wrote:
>On Wed, 11 Jan 2017, Phil Reid wrote:
>
>> Oops, title should be PATCH V2.
>> 
>> On 11/01/2017 14:51, Phil Reid wrote:
>> > This adds TI's tlc4541 16-bit ADC driver. Which is a single channel
>> > ADC. Supports raw and trigger buffer access.
>> > Also supports the tlc3541 14-bit device, which has not been tested.
>> > Implementation of the tlc3541 is fairly straight forward thou.
>
>comments below
>
>> 
>> > Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
>> > ---
>> > 
>> > Notes:
>> >     Changes from v1:
>> >     - Add tlc3541 support and chan spec.
>> >     - remove fields that where already 0 from TLC4541_V_CHAN macro
>> >     - Increase rx_buf size in tlc4541_state to avoid copy in
>> > tlc4541_trigger_handle
>> >     - Remove erroneous be16_to_cpu in tlc4541_trigger_handle
>> >     - Docs/binding: spi -> SPI & add ti,tlc3541
>> > 
>> >     I haven't add Rob's Ack due to adding a new compatible string.
>> > 
>> >     I tried to ".index = 1" from the spec as suggested by Peter,
>but that
>> > didn't
>> >     seem to work. Perhaps remove of .channel was the intended
>target.
>
>the only between index = 0/1 should be that the channel is called
>in_voltage0_raw vs in_voltage_raw in sysfs -- maybe there is an issue
>in 
>iio_readdev?
> 
>> >     Example output from iio_readdev
>> > 
>> >     with ".index = 1"
>> >     root@cyclone5:~# mkdir
>/sys/kernel/config/iio/triggers/hrtimer/hr1
>> >     root@cyclone5:~# iio_readdev -t hr1 -b 32 -s 10 tlc4541 |
>hexdump
>> >     WARNING: High-speed mode not enabled
>> >     0000000 af00 0000 0000 0000 b922 ca99 93da 1492
>> >     0000010 a800 00ff 0000 0000 b246 cb30 93da 1492
>> >     0000020 a900 0000 0000 0000 4f9c cbc9 93da 1492
>> >     0000030 aa00 00ff 0000 0000 bd2c cc61 93da 1492
>> >     0000040 aa00 00ff 0000 0000 544c ccfa 93da 1492
>> >     0000050 ab00 00ff 0000 0000 e806 cd92 93da 1492
>> >     0000060 a900 00ff 0000 0000 846c ce2b 93da 1492
>> >     0000070 ab00 0000 0000 0000 2efc cec8 93da 1492
>> >     0000080 a800 00ff 0000 0000 b090 cf5c 93da 1492
>> >     0000090 a900 00ff 0000 0000 476a cff5 93da 1492
>> > 
>> >     without .index
>> >     root@cyclone5:~# mkdir
>/sys/kernel/config/iio/triggers/hrtimer/hr1
>> >     root@cyclone5:~# iio_readdev -t hr1 -b 32 -s 10 tlc4541 |
>hexdump
>> >     WARNING: High-speed mode not enabled
>> >     0000000 6db0 eeb6 93e3 1492 35e0 ef4f 93e3 1492
>> >     0000010 4b34 efe5 93e3 1492 e9f2 f07d 93e3 1492
>> >     0000020 6182 f116 93e3 1492 090a f1af 93e3 1492
>> >     0000030 409c f249 93e3 1492 6c1a f2e0 93e3 1492
>> >     0000040 cd02 f378 93e3 1492 9582 f411 93e3 1492
>> > 
>> >  .../devicetree/bindings/iio/adc/ti-tlc4541.txt     |  17 ++
>> >  drivers/iio/adc/Kconfig                            |  11 +
>> >  drivers/iio/adc/Makefile                           |   1 +
>> >  drivers/iio/adc/ti-tlc4541.c                       | 276
>> > +++++++++++++++++++++
>> >  4 files changed, 305 insertions(+)
>> >  create mode 100644
>Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>> >  create mode 100644 drivers/iio/adc/ti-tlc4541.c
>> > 
>> > diff --git
>a/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>> > b/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>> > new file mode 100644
>> > index 0000000..e1de2bd
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>> > @@ -0,0 +1,17 @@
>> > +* Texas Instruments' TLC4541
>> > +
>> > +Required properties:
>> > + - compatible: Should be one of
>> > +	* "ti,tlc4541"
>> > +	* "ti,tlc3541"
>> > +	- reg: SPI chip select number for the device
>> > + - vref-supply: The regulator supply for ADC reference voltage
>> > + - spi-max-frequency: Max SPI frequency to use (<= 200000)
>> > +
>> > +Example:
>> > +adc@0 {
>> > +	compatible = "ti,adc0832";
>
>pasto here, should be ti,tlc4541 probably
>
>> > +	reg = <0>;
>> > +	vref-supply = <&vdd_supply>;
>> > +	spi-max-frequency = <200000>;
>> > +};
>> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> > index 99c0514..4dda3f0 100644
>> > --- a/drivers/iio/adc/Kconfig
>> > +++ b/drivers/iio/adc/Kconfig
>> > @@ -525,6 +525,17 @@ config TI_AM335X_ADC
>> >  	  To compile this driver as a module, choose M here: the module
>will
>> > be
>> >  	  called ti_am335x_adc.
>> > 
>> > +config TI_TLC4541
>> > +	tristate "Texas Instruments TLC4541 ADC driver"
>> > +	depends on SPI
>> > +	select IIO_BUFFER
>> > +	select IIO_TRIGGERED_BUFFER
>> > +	help
>> > +	  Say yes here to build support for Texas Instruments TLC4541 ADC
>
>mention TLC3541 here as well?
>
>> > chip.
>> > +
>> > +	  This driver can also be built as a module. If so, the module
>will be
>> > +	  called ti-tlc4541.
>> > +
>> >  config TWL4030_MADC
>> >  	tristate "TWL4030 MADC (Monitoring A/D Converter)"
>> >  	depends on TWL4030_CORE
>> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> > index 7a40c04..9bf2377 100644
>> > --- a/drivers/iio/adc/Makefile
>> > +++ b/drivers/iio/adc/Makefile
>> > @@ -49,6 +49,7 @@ obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
>> >  obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
>> >  obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
>> >  obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>> > +obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
>> >  obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
>> >  obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
>> >  obj-$(CONFIG_VF610_ADC) += vf610_adc.o
>> > diff --git a/drivers/iio/adc/ti-tlc4541.c
>b/drivers/iio/adc/ti-tlc4541.c
>> > new file mode 100644
>> > index 0000000..a0cd5e1
>> > --- /dev/null
>> > +++ b/drivers/iio/adc/ti-tlc4541.c
>> > @@ -0,0 +1,276 @@
>> > +/*
>> > + * TI tlc4541 ADC Driver
>> > + *
>> > + * Copyright (C) 2017 Phil Reid
>> > + *
>> > + * Datasheets can be found here:
>> > + * http://www.ti.com/lit/gpn/tlc3541
>> > + * http://www.ti.com/lit/gpn/tlc4541
>> > + *
>> > + * 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.
>> > + *
>> > + * The tlc4541 requires 24 clock cycles to start a transfer.
>> > + * Conversion then takes 2.94us to complete before data is ready
>> > + * Data is returned MSB first.
>> > + */
>> > +
>> > +#include <linux/delay.h>
>> > +#include <linux/device.h>
>> > +#include <linux/err.h>
>> > +#include <linux/interrupt.h>
>> > +#include <linux/iio/iio.h>
>> > +#include <linux/iio/sysfs.h>
>> > +#include <linux/iio/buffer.h>
>> > +#include <linux/iio/trigger_consumer.h>
>> > +#include <linux/iio/triggered_buffer.h>
>> > +#include <linux/kernel.h>
>> > +#include <linux/module.h>
>> > +#include <linux/regulator/consumer.h>
>> > +#include <linux/slab.h>
>> > +#include <linux/spi/spi.h>
>> > +#include <linux/sysfs.h>
>> > +
>> > +struct tlc4541_state {
>> > +	struct spi_device               *spi;
>> > +	struct regulator                *reg;
>> > +	struct spi_transfer             scan_single_xfer[3];
>> > +	struct spi_message              scan_single_msg;
>> > +
>> > +	/*
>> > +	 * DMA (thus cache coherency maintenance) requires the
>> > +	 * transfer buffers to live in their own cache lines.
>> > +	 * 2 bytes data + 6 bytes padding + 8 bytes timestamp when
>> > +	 * call iio_push_to_buffers_with_timestamp.
>> > +	 */
>> > +	__be16                          rx_buf[8] ____cacheline_aligned;
>> > +};
>> > +
>> > +struct tlc4541_chip_info {
>> > +	const struct iio_chan_spec *channels;
>> > +	unsigned int num_channels;
>> > +};
>> > +
>> > +enum tlc4541_id {
>> > +	TLC3541,
>> > +	TLC4541,
>> > +};
>> > +
>> > +#define TLC4541_V_CHAN(bits, bitshift) {                          
>   \
>> > +		.type = IIO_VOLTAGE,                                  \
>> > +		.indexed = 1,                                         \
>
>shouldn't be needed
>
>> > +		.info_mask_separate       = BIT(IIO_CHAN_INFO_RAW),   \
>> > +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>> > +		.scan_type = {                                        \
>> > +			.sign = 'u',                                  \
>> > +			.realbits = (bits),                           \
>> > +			.storagebits = 16,                            \
>> > +			.shift = bitshift,                            \
>
>(bitshift)
>
>> > +			.endianness = IIO_BE,                         \
>> > +		},                                                    \
>> > +	}
>> > +
>> > +#define DECLARE_TLC4541_CHANNELS(name, bits, bitshift) \
>> > +const struct iio_chan_spec name ## _channels[] = { \
>> > +	TLC4541_V_CHAN(bits, bitshift), \
>> > +	IIO_CHAN_SOFT_TIMESTAMP(1), \
>> > +}
>> > +
>> > +static DECLARE_TLC4541_CHANNELS(tlc4541, 16, 0);
>> > +static DECLARE_TLC4541_CHANNELS(tlc3541, 14, 2);
>
>maybe always keep the chip variants in the same order, the enum has
>3541 
>first
>
>> > +
>> > +static const struct tlc4541_chip_info tlc4541_chip_info[] = {
>> > +	[TLC4541] = {
>> > +		.channels = tlc4541_channels,
>> > +		.num_channels = ARRAY_SIZE(tlc4541_channels),
>> > +	},
>> > +	[TLC3541] = {
>> > +		.channels = tlc3541_channels,
>> > +		.num_channels = ARRAY_SIZE(tlc3541_channels),
>> > +	},
>> > +};
>> > +
>> > +static irqreturn_t tlc4541_trigger_handler(int irq, void *p)
>> > +{
>> > +	struct iio_poll_func *pf = p;
>> > +	struct iio_dev *indio_dev = pf->indio_dev;
>> > +	struct tlc4541_state *st = iio_priv(indio_dev);
>> > +	int ret;
>> > +
>> > +	ret = spi_sync(st->spi, &st->scan_single_msg);
>> > +	if (ret < 0)
>> > +		goto done;
>> > +
>> > +	iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
>> > +					   iio_get_time_ns(indio_dev));
>> > +
>> > +done:
>> > +	iio_trigger_notify_done(indio_dev->trig);
>> > +	return IRQ_HANDLED;
>> > +}
>> > +
>> > +static int tlc4541_get_range(struct tlc4541_state *st)
>> > +{
>> > +	int vref;
>> > +
>> > +	vref = regulator_get_voltage(st->reg);
>> > +	if (vref < 0)
>> > +		return vref;
>> > +
>> > +	vref /= 1000;
>> > +
>> > +	return vref;
>> > +}
>> > +
>> > +static int tlc4541_read_raw(struct iio_dev *indio_dev,
>> > +			    struct iio_chan_spec const *chan,
>> > +			    int *val,
>> > +			    int *val2,
>> > +			    long m)
>> > +{
>> > +	int ret = 0;
>> > +	struct tlc4541_state *st = iio_priv(indio_dev);
>> > +
>> > +	switch (m) {
>> > +	case IIO_CHAN_INFO_RAW:
>> > +		ret = iio_device_claim_direct_mode(indio_dev);
>> > +		if (ret)
>> > +			return ret;
>> > +		ret = spi_sync(st->spi, &st->scan_single_msg);
>> > +		iio_device_release_direct_mode(indio_dev);
>> > +		if (ret < 0)
>> > +			return ret;
>> > +		*val = be16_to_cpu(st->rx_buf[0]);
>> > +		*val = *val >> chan->scan_type.shift;
>> > +		*val &= GENMASK(chan->scan_type.realbits - 1, 0);
>
>is the GENMASK() necessary?, the trigger handler doesn't do it
>
>> > +		return IIO_VAL_INT;
>> > +	case IIO_CHAN_INFO_SCALE:
>> > +		ret = tlc4541_get_range(st);
>> > +		if (ret < 0)
>> > +			return ret;
>> > +		*val = ret;
>> > +		*val2 = chan->scan_type.realbits;
>> > +		return IIO_VAL_FRACTIONAL_LOG2;
>> > +	}
>> > +	return -EINVAL;
>> > +}
>> > +
>> > +static const struct iio_info tlc4541_info = {
>> > +	.read_raw = &tlc4541_read_raw,
>> > +	.driver_module = THIS_MODULE,
>> > +};
>> > +
>> > +static int tlc4541_probe(struct spi_device *spi)
>> > +{
>> > +	struct tlc4541_state *st;
>> > +	struct iio_dev *indio_dev;
>> > +	const struct tlc4541_chip_info *info;
>> > +	int ret;
>> > +	int8_t device_init = 0;
>> > +
>> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>
>wondering what happens to the cache aligned rx_buf here?
Fine as we are careful to also align the private data so this works.
>
>> > +	if (indio_dev == NULL)
>> > +		return -ENOMEM;
>> > +
>> > +	st = iio_priv(indio_dev);
>> > +
>> > +	spi_set_drvdata(spi, indio_dev);
>> > +
>> > +	st->spi = spi;
>> > +
>> > +	info = &tlc4541_chip_info[spi_get_device_id(spi)->driver_data];
>> > +
>> > +	indio_dev->name = spi_get_device_id(spi)->name;
>> > +	indio_dev->dev.parent = &spi->dev;
>> > +	indio_dev->modes = INDIO_DIRECT_MODE;
>> > +	indio_dev->channels = info->channels;
>> > +	indio_dev->num_channels = info->num_channels;
>> > +	indio_dev->info = &tlc4541_info;
>> > +
>> > +	/* perform reset */
>> > +	spi_write(spi, &device_init, 1);
>> > +
>> > +	/* Setup default message */
>> > +	st->scan_single_xfer[0].rx_buf = &st->rx_buf[0];
>> > +	st->scan_single_xfer[0].len = 3;
>> > +	st->scan_single_xfer[1].delay_usecs = 3;
>> > +	st->scan_single_xfer[2].rx_buf = &st->rx_buf[0];
>> > +	st->scan_single_xfer[2].len = 2;
>> > +
>> > +	spi_message_init(&st->scan_single_msg);
>> > +	spi_message_add_tail(&st->scan_single_xfer[0],
>&st->scan_single_msg);
>> > +	spi_message_add_tail(&st->scan_single_xfer[1],
>&st->scan_single_msg);
>> > +	spi_message_add_tail(&st->scan_single_xfer[2],
>&st->scan_single_msg);
>> > +
>> > +	st->reg = devm_regulator_get(&spi->dev, "vref");
>> > +	if (IS_ERR(st->reg))
>> > +		return PTR_ERR(st->reg);
>> > +
>> > +	ret = regulator_enable(st->reg);
>> > +	if (ret)
>> > +		return ret;
>> > +
>> > +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> > +			&tlc4541_trigger_handler, NULL);
>> > +	if (ret)
>> > +		goto error_disable_reg;
>> > +
>> > +	ret = iio_device_register(indio_dev);
>> > +	if (ret)
>> > +		goto error_cleanup_buffer;
>> > +
>> > +	return 0;
>> > +
>> > +error_cleanup_buffer:
>> > +	iio_triggered_buffer_cleanup(indio_dev);
>> > +error_disable_reg:
>> > +	regulator_disable(st->reg);
>> > +
>> > +	return ret;
>> > +}
>> > +
>> > +static int tlc4541_remove(struct spi_device *spi)
>> > +{
>> > +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> > +	struct tlc4541_state *st = iio_priv(indio_dev);
>> > +
>> > +	iio_device_unregister(indio_dev);
>> > +	iio_triggered_buffer_cleanup(indio_dev);
>> > +	regulator_disable(st->reg);
>> > +
>> > +	return 0;
>> > +}
>> > +
>> > +#ifdef CONFIG_OF
>
>maybe drop the newlines here?
>
>> > +
>> > +static const struct of_device_id tlc4541_dt_ids[] = {
>> > +	{ .compatible = "ti,tlc3541", },
>> > +	{ .compatible = "ti,tlc4541", },
>> > +	{}
>> > +};
>> > +MODULE_DEVICE_TABLE(of, tlc4541_dt_ids);
>> > +
>> > +#endif
>> > +
>> > +static const struct spi_device_id tlc4541_id[] = {
>> > +	{"tlc3541", TLC3541},
>> > +	{"tlc4541", TLC4541},
>> > +	{}
>> > +};
>> > +MODULE_DEVICE_TABLE(spi, tlc4541_id);
>> > +
>> > +static struct spi_driver tlc4541_driver = {
>> > +	.driver = {
>> > +		.name   = "tlc4541",
>> > +		.of_match_table = of_match_ptr(tlc4541_dt_ids),
>> > +	},
>> > +	.probe          = tlc4541_probe,
>> > +	.remove         = tlc4541_remove,
>> > +	.id_table       = tlc4541_id,
>> > +};
>> > +module_spi_driver(tlc4541_driver);
>> > +
>> > +MODULE_AUTHOR("Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>");
>> > +MODULE_DESCRIPTION("Texas Instruments TLC4541 ADC");
>> > +MODULE_LICENSE("GPL v2");
>> > 
>> 
>> 
>> 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply

* Re: [PATCH v1 1/1] iio: adc: tlc4541: add support for TI tlc4541 adc
From: Jonathan Cameron @ 2017-01-11 14:52 UTC (permalink / raw)
  To: Phil Reid, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484117471-67082-1-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>



On 11 January 2017 06:51:11 GMT+00:00, Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org> wrote:
>This adds TI's tlc4541 16-bit ADC driver. Which is a single channel
>ADC. Supports raw and trigger buffer access.
>Also supports the tlc3541 14-bit device, which has not been tested.
>Implementation of the tlc3541 is fairly straight forward thou.
>
>Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
One minor suggestion from me.

J
>---
>
>Notes:
>    Changes from v1:
>    - Add tlc3541 support and chan spec.
>    - remove fields that where already 0 from TLC4541_V_CHAN macro
>- Increase rx_buf size in tlc4541_state to avoid copy in
>tlc4541_trigger_handle
>    - Remove erroneous be16_to_cpu in tlc4541_trigger_handle
>    - Docs/binding: spi -> SPI & add ti,tlc3541
>    
>    I haven't add Rob's Ack due to adding a new compatible string.
>    
>I tried to ".index = 1" from the spec as suggested by Peter, but that
>didn't
>    seem to work. Perhaps remove of .channel was the intended target.
>    
>    Example output from iio_readdev
>    
>    with ".index = 1"
>    root@cyclone5:~# mkdir /sys/kernel/config/iio/triggers/hrtimer/hr1
>    root@cyclone5:~# iio_readdev -t hr1 -b 32 -s 10 tlc4541 | hexdump
>    WARNING: High-speed mode not enabled
>    0000000 af00 0000 0000 0000 b922 ca99 93da 1492
>    0000010 a800 00ff 0000 0000 b246 cb30 93da 1492
>    0000020 a900 0000 0000 0000 4f9c cbc9 93da 1492
>    0000030 aa00 00ff 0000 0000 bd2c cc61 93da 1492
>    0000040 aa00 00ff 0000 0000 544c ccfa 93da 1492
>    0000050 ab00 00ff 0000 0000 e806 cd92 93da 1492
>    0000060 a900 00ff 0000 0000 846c ce2b 93da 1492
>    0000070 ab00 0000 0000 0000 2efc cec8 93da 1492
>    0000080 a800 00ff 0000 0000 b090 cf5c 93da 1492
>    0000090 a900 00ff 0000 0000 476a cff5 93da 1492
>    
>    without .index
>    root@cyclone5:~# mkdir /sys/kernel/config/iio/triggers/hrtimer/hr1
>    root@cyclone5:~# iio_readdev -t hr1 -b 32 -s 10 tlc4541 | hexdump
>    WARNING: High-speed mode not enabled
>    0000000 6db0 eeb6 93e3 1492 35e0 ef4f 93e3 1492
>    0000010 4b34 efe5 93e3 1492 e9f2 f07d 93e3 1492
>    0000020 6182 f116 93e3 1492 090a f1af 93e3 1492
>    0000030 409c f249 93e3 1492 6c1a f2e0 93e3 1492
>    0000040 cd02 f378 93e3 1492 9582 f411 93e3 1492
>
> .../devicetree/bindings/iio/adc/ti-tlc4541.txt     |  17 ++
> drivers/iio/adc/Kconfig                            |  11 +
> drivers/iio/adc/Makefile                           |   1 +
>drivers/iio/adc/ti-tlc4541.c                       | 276
>+++++++++++++++++++++
> 4 files changed, 305 insertions(+)
>create mode 100644
>Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
> create mode 100644 drivers/iio/adc/ti-tlc4541.c
>
>diff --git a/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>b/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>new file mode 100644
>index 0000000..e1de2bd
>--- /dev/null
>+++ b/Documentation/devicetree/bindings/iio/adc/ti-tlc4541.txt
>@@ -0,0 +1,17 @@
>+* Texas Instruments' TLC4541
>+
>+Required properties:
>+ - compatible: Should be one of
>+	* "ti,tlc4541"
>+	* "ti,tlc3541"
>+	- reg: SPI chip select number for the device
>+ - vref-supply: The regulator supply for ADC reference voltage
>+ - spi-max-frequency: Max SPI frequency to use (<= 200000)
>+
>+Example:
>+adc@0 {
>+	compatible = "ti,adc0832";
>+	reg = <0>;
>+	vref-supply = <&vdd_supply>;
>+	spi-max-frequency = <200000>;
>+};
>diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>index 99c0514..4dda3f0 100644
>--- a/drivers/iio/adc/Kconfig
>+++ b/drivers/iio/adc/Kconfig
>@@ -525,6 +525,17 @@ config TI_AM335X_ADC
>	  To compile this driver as a module, choose M here: the module will
>be
> 	  called ti_am335x_adc.
> 
>+config TI_TLC4541
>+	tristate "Texas Instruments TLC4541 ADC driver"
>+	depends on SPI
>+	select IIO_BUFFER
>+	select IIO_TRIGGERED_BUFFER
>+	help
>+	  Say yes here to build support for Texas Instruments TLC4541 ADC
>chip.
>+
>+	  This driver can also be built as a module. If so, the module will
>be
>+	  called ti-tlc4541.
>+
> config TWL4030_MADC
> 	tristate "TWL4030 MADC (Monitoring A/D Converter)"
> 	depends on TWL4030_CORE
>diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>index 7a40c04..9bf2377 100644
>--- a/drivers/iio/adc/Makefile
>+++ b/drivers/iio/adc/Makefile
>@@ -49,6 +49,7 @@ obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
> obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
> obj-$(CONFIG_TI_ADS8688) += ti-ads8688.o
> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
>+obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
> obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
> obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
> obj-$(CONFIG_VF610_ADC) += vf610_adc.o
>diff --git a/drivers/iio/adc/ti-tlc4541.c
>b/drivers/iio/adc/ti-tlc4541.c
>new file mode 100644
>index 0000000..a0cd5e1
>--- /dev/null
>+++ b/drivers/iio/adc/ti-tlc4541.c
>@@ -0,0 +1,276 @@
>+/*
>+ * TI tlc4541 ADC Driver
>+ *
>+ * Copyright (C) 2017 Phil Reid
>+ *
>+ * Datasheets can be found here:
>+ * http://www.ti.com/lit/gpn/tlc3541
>+ * http://www.ti.com/lit/gpn/tlc4541
>+ *
>+ * 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.
>+ *
>+ * The tlc4541 requires 24 clock cycles to start a transfer.
>+ * Conversion then takes 2.94us to complete before data is ready
>+ * Data is returned MSB first.
>+ */
>+
>+#include <linux/delay.h>
>+#include <linux/device.h>
>+#include <linux/err.h>
>+#include <linux/interrupt.h>
>+#include <linux/iio/iio.h>
>+#include <linux/iio/sysfs.h>
>+#include <linux/iio/buffer.h>
>+#include <linux/iio/trigger_consumer.h>
>+#include <linux/iio/triggered_buffer.h>
>+#include <linux/kernel.h>
>+#include <linux/module.h>
>+#include <linux/regulator/consumer.h>
>+#include <linux/slab.h>
>+#include <linux/spi/spi.h>
>+#include <linux/sysfs.h>
>+
>+struct tlc4541_state {
>+	struct spi_device               *spi;
>+	struct regulator                *reg;
>+	struct spi_transfer             scan_single_xfer[3];
>+	struct spi_message              scan_single_msg;
>+
>+	/*
>+	 * DMA (thus cache coherency maintenance) requires the
>+	 * transfer buffers to live in their own cache lines.
>+	 * 2 bytes data + 6 bytes padding + 8 bytes timestamp when
>+	 * call iio_push_to_buffers_with_timestamp.
>+	 */
>+	__be16                          rx_buf[8] ____cacheline_aligned;
>+};
>+
>+struct tlc4541_chip_info {
>+	const struct iio_chan_spec *channels;
>+	unsigned int num_channels;
>+};
>+
>+enum tlc4541_id {
>+	TLC3541,
>+	TLC4541,
>+};
>+
>+#define TLC4541_V_CHAN(bits, bitshift) {                             
>\
>+		.type = IIO_VOLTAGE,                                  \
>+		.indexed = 1,                                         \
>+		.info_mask_separate       = BIT(IIO_CHAN_INFO_RAW),   \
>+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
>+		.scan_type = {                                        \
>+			.sign = 'u',                                  \
>+			.realbits = (bits),                           \
>+			.storagebits = 16,                            \
>+			.shift = bitshift,                            \
>+			.endianness = IIO_BE,                         \
>+		},                                                    \
>+	}
>+
>+#define DECLARE_TLC4541_CHANNELS(name, bits, bitshift) \
>+const struct iio_chan_spec name ## _channels[] = { \
>+	TLC4541_V_CHAN(bits, bitshift), \
>+	IIO_CHAN_SOFT_TIMESTAMP(1), \
>+}
>+
>+static DECLARE_TLC4541_CHANNELS(tlc4541, 16, 0);
>+static DECLARE_TLC4541_CHANNELS(tlc3541, 14, 2);
>+
>+static const struct tlc4541_chip_info tlc4541_chip_info[] = {
>+	[TLC4541] = {
>+		.channels = tlc4541_channels,
>+		.num_channels = ARRAY_SIZE(tlc4541_channels),
>+	},
>+	[TLC3541] = {
>+		.channels = tlc3541_channels,
>+		.num_channels = ARRAY_SIZE(tlc3541_channels),
>+	},
>+};
>+
>+static irqreturn_t tlc4541_trigger_handler(int irq, void *p)
>+{
>+	struct iio_poll_func *pf = p;
>+	struct iio_dev *indio_dev = pf->indio_dev;
>+	struct tlc4541_state *st = iio_priv(indio_dev);
>+	int ret;
>+
>+	ret = spi_sync(st->spi, &st->scan_single_msg);
>+	if (ret < 0)
>+		goto done;
>+
>+	iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
>+					   iio_get_time_ns(indio_dev));
>+
>+done:
>+	iio_trigger_notify_done(indio_dev->trig);
>+	return IRQ_HANDLED;
>+}
>+
>+static int tlc4541_get_range(struct tlc4541_state *st)
>+{
>+	int vref;
>+
>+	vref = regulator_get_voltage(st->reg);
>+	if (vref < 0)
>+		return vref;
>+
>+	vref /= 1000;
>+
>+	return vref;
>+}
>+
>+static int tlc4541_read_raw(struct iio_dev *indio_dev,
>+			    struct iio_chan_spec const *chan,
>+			    int *val,
>+			    int *val2,
>+			    long m)
>+{
>+	int ret = 0;
>+	struct tlc4541_state *st = iio_priv(indio_dev);
>+
>+	switch (m) {
>+	case IIO_CHAN_INFO_RAW:
>+		ret = iio_device_claim_direct_mode(indio_dev);
>+		if (ret)
>+			return ret;
>+		ret = spi_sync(st->spi, &st->scan_single_msg);
>+		iio_device_release_direct_mode(indio_dev);
>+		if (ret < 0)
>+			return ret;
>+		*val = be16_to_cpu(st->rx_buf[0]);
>+		*val = *val >> chan->scan_type.shift;
>+		*val &= GENMASK(chan->scan_type.realbits - 1, 0);
>+		return IIO_VAL_INT;
>+	case IIO_CHAN_INFO_SCALE:
>+		ret = tlc4541_get_range(st);
>+		if (ret < 0)
>+			return ret;
>+		*val = ret;
>+		*val2 = chan->scan_type.realbits;
>+		return IIO_VAL_FRACTIONAL_LOG2;
>+	}
>+	return -EINVAL;
>+}
>+
>+static const struct iio_info tlc4541_info = {
>+	.read_raw = &tlc4541_read_raw,
>+	.driver_module = THIS_MODULE,
>+};
>+
>+static int tlc4541_probe(struct spi_device *spi)
>+{
>+	struct tlc4541_state *st;
>+	struct iio_dev *indio_dev;
>+	const struct tlc4541_chip_info *info;
>+	int ret;
>+	int8_t device_init = 0;
>+
>+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
>+	if (indio_dev == NULL)
>+		return -ENOMEM;
>+
>+	st = iio_priv(indio_dev);
>+
>+	spi_set_drvdata(spi, indio_dev);
>+
>+	st->spi = spi;
>+
>+	info = &tlc4541_chip_info[spi_get_device_id(spi)->driver_data];
>+
>+	indio_dev->name = spi_get_device_id(spi)->name;
>+	indio_dev->dev.parent = &spi->dev;
>+	indio_dev->modes = INDIO_DIRECT_MODE;
>+	indio_dev->channels = info->channels;
>+	indio_dev->num_channels = info->num_channels;
>+	indio_dev->info = &tlc4541_info;
>+
>+	/* perform reset */
>+	spi_write(spi, &device_init, 1);
>+
>+	/* Setup default message */
>+	st->scan_single_xfer[0].rx_buf = &st->rx_buf[0];
>+	st->scan_single_xfer[0].len = 3;
>+	st->scan_single_xfer[1].delay_usecs = 3;
>+	st->scan_single_xfer[2].rx_buf = &st->rx_buf[0];
>+	st->scan_single_xfer[2].len = 2;
>+
>+	spi_message_init(&st->scan_single_msg);
>+	spi_message_add_tail(&st->scan_single_xfer[0], &st->scan_single_msg);
>+	spi_message_add_tail(&st->scan_single_xfer[1], &st->scan_single_msg);
>+	spi_message_add_tail(&st->scan_single_xfer[2], &st->scan_single_msg);
spi_init_with_transfers.
>+
>+	st->reg = devm_regulator_get(&spi->dev, "vref");
>+	if (IS_ERR(st->reg))
>+		return PTR_ERR(st->reg);
>+
>+	ret = regulator_enable(st->reg);
>+	if (ret)
>+		return ret;
>+
>+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>+			&tlc4541_trigger_handler, NULL);
>+	if (ret)
>+		goto error_disable_reg;
>+
>+	ret = iio_device_register(indio_dev);
>+	if (ret)
>+		goto error_cleanup_buffer;
>+
>+	return 0;
>+
>+error_cleanup_buffer:
>+	iio_triggered_buffer_cleanup(indio_dev);
>+error_disable_reg:
>+	regulator_disable(st->reg);
>+
>+	return ret;
>+}
>+
>+static int tlc4541_remove(struct spi_device *spi)
>+{
>+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>+	struct tlc4541_state *st = iio_priv(indio_dev);
>+
>+	iio_device_unregister(indio_dev);
>+	iio_triggered_buffer_cleanup(indio_dev);
>+	regulator_disable(st->reg);
>+
>+	return 0;
>+}
>+
>+#ifdef CONFIG_OF
>+
>+static const struct of_device_id tlc4541_dt_ids[] = {
>+	{ .compatible = "ti,tlc3541", },
>+	{ .compatible = "ti,tlc4541", },
>+	{}
>+};
>+MODULE_DEVICE_TABLE(of, tlc4541_dt_ids);
>+
>+#endif
>+
>+static const struct spi_device_id tlc4541_id[] = {
>+	{"tlc3541", TLC3541},
>+	{"tlc4541", TLC4541},
>+	{}
>+};
>+MODULE_DEVICE_TABLE(spi, tlc4541_id);
>+
>+static struct spi_driver tlc4541_driver = {
>+	.driver = {
>+		.name   = "tlc4541",
>+		.of_match_table = of_match_ptr(tlc4541_dt_ids),
>+	},
>+	.probe          = tlc4541_probe,
>+	.remove         = tlc4541_remove,
>+	.id_table       = tlc4541_id,
>+};
>+module_spi_driver(tlc4541_driver);
>+
>+MODULE_AUTHOR("Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>");
>+MODULE_DESCRIPTION("Texas Instruments TLC4541 ADC");
>+MODULE_LICENSE("GPL v2");

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply

* [PATCH] Documentation: dt: dwc3: add reference to the usb-xhci properties
From: Martin Blumenstingl @ 2017-01-11 14:59 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
  Cc: mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	Martin Blumenstingl

dwc3 internally creates a usb-xhci device which means that all
properties documented in usb-xhci.txt are supported as well.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/dwc3.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt
index e3e6983288e3..f658f394c2d3 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -56,6 +56,10 @@ Optional properties:
 
  - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be reallocated.
 
+ - in addition all properties from usb-xhci.txt from the current directory are
+   supported as well
+
+
 This is usually a subnode to DWC3 glue to which it is connected.
 
 dwc3@4a030000 {
-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 v2 2/3] gpio: Add gpio driver support for ThunderX and OCTEON-TX
From: Linus Walleij @ 2017-01-11 15:07 UTC (permalink / raw)
  To: David Daney
  Cc: David Daney, Alexandre Courbot, Rob Herring, Mark Rutland,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, David Daney, Marc Zyngier
In-Reply-To: <245af7b3-cfb7-e606-f1b8-f68b2ec0b8d1@caviumnetworks.com>

On Mon, Jan 9, 2017 at 9:02 PM, David Daney <ddaney@caviumnetworks.com> wrote:

>> if (test_bit(line, gpio->invert_mask))
>>    return !(read_bits & BIT(bank_bit));
>> else
>>    return !!(read_bits & BIT(bank_bit));
>>
>> OK maybe not much clearer but seems clearer to me.
>
> As I really dislike the "!!" idiom, would you settle for:
>
>  if (test_bit(line, gpio->invert_mask))
>     return (read_bits & BIT(bank_bit)) == 0;
>  else
>     return (read_bits & BIT(bank_bit)) != 0;

Not the biggest issue in the world. But I maintain a huge stack
of GPIO drivers and it drives me crazy that each one has to bear
the mark of the authors habits rather than mine.

>> I think this is overkill. Use hierarchical irqdomain.
>
> I will look into it.  I suspect it will require more lines of driver code to
> implement it than what I have here (that does actually work).

I understand. But at the same time, the kernel needs to have the
right idea of what it is dealing with here.

The generic IRQ handling code will take a shorter fastpath if
you are using hierarchical irqdomain (I think?) but I can't claim
to be an expert. When in doubt, consult Marc Z.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH] pinctrl: qcom: Add msm8998 pinctrl driver
From: Linus Walleij @ 2017-01-11 15:12 UTC (permalink / raw)
  To: Imran Khan, Bjorn Andersson
  Cc: Andy Gross, Rob Herring, Mark Rutland, David Brown,
	open list:PIN CONTROL SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	open list, open list:ARM/QUALCOMM SUPPORT,
	open list:ARM/QUALCOMM SUPPORT
In-Reply-To: <1483974019-8235-1-git-send-email-kimran@codeaurora.org>

On Mon, Jan 9, 2017 at 4:00 PM, Imran Khan <kimran@codeaurora.org> wrote:

> Add initial pinctrl driver to support pin configuration with
> pinctrl framework for msm8998.
>
> Signed-off-by: Imran Khan <kimran@codeaurora.org>

You need review from the Qcom pinctrl maintainer Bjorn Andersson
for this patch.


+#define NORTH  0x500000
+#define WEST   0x100000
+#define EAST   0x900000
(...)
> +static const struct msm_pingroup msm8998_groups[] = {
> +       PINGROUP(0, EAST, blsp_spi1, blsp_uart1_a, blsp_uim1_a, NA, NA, NA, NA,
(...)
> +       PINGROUP(6, WEST, blsp_spi8, blsp_uart8_a, blsp_i2c8, NA, NA, NA, NA,
(...)
> +       PINGROUP(35, NORTH, pci_e0, jitter_bist, NA, NA, NA, NA, NA, NA, NA),
(...)

No south? :)

Are these the left/right/top edges of the chip or a reference
to x86 bridges terminology? It warrants that you add a comment
to the driver file explaining what it means.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 3/5] pinctrl: sunxi: add driver for V3s SoC
From: Linus Walleij @ 2017-01-11 15:24 UTC (permalink / raw)
  To: Icenowy Zheng
  Cc: Maxime Ripard, Chen-Yu Tsai, Stephen Boyd,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-clk,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-sunxi
In-Reply-To: <20170103151629.19447-4-icenowy-ymACFijhrKM@public.gmane.org>

On Tue, Jan 3, 2017 at 4:16 PM, Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org> wrote:

> V3s SoC features only a pin controller (for the lack of CPUs part).
>
> Add a driver for this controller.
>
> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>

Patch applied with Maxime's ACK.

Yours,
Linus Walleij

^ permalink raw reply

* [RFC PATCH 0/2] initialize (multiple) PHYs in xhci-plat
From: Martin Blumenstingl @ 2017-01-11 15:29 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, mathias.nyman-ral2JQCrhuEAvxtiuMwx3w,
	vegard.nossum-QHcLZuEGTsvQT0dZR+AlfA,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
	treding-DDmLM1+adcrQT0dZR+AlfA, geert-Td1EMuHUCqxL1ZNQvxDV9g,
	manjunath.goudar-QSEj5FYQhm4dnm+yROfE0A, arnd-r2nGTMty4D4,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, ahaslam-rdvid1DuHRBWk0Htik3J/w,
	Martin Blumenstingl

This series is the outcome of a discussion with Felipe Balbi,
see [0] and [1].
The quick-summary of this is:
- dwc3 already takes one USB2 and one USB3 PHY and initializes these
  correct
- some other HCI platform drivers (like ehci-platform.c, xhci-mtk.c and
  ohci-platform.c) do not have a limitation on the number of PHYs - they
  support one PHY per actual host port
- Amlogic Meson GXL and GXM SoCs come with a dwc3 IP block which has two
  or three USB2 ports enabled on the internal root-hub. The SoCs also
  provide separate USB2 PHYs, one per port. All USB2 PHYs (which are
  internally "connected" to the dwc3 roothub) need to be powered on,
  otherwise USB devices cannot be enumerated (even if just one PHY is
  disabled and if the device is plugged into another, enabled port)

In my first attempt to get USB supported on the GXL and GXM SoCs I tried
to work-around the problem that I could not pass multiple PHYs to the
dwc3 controller.
This was rejected by Rob Herring (which was definitely the thing to do in
my opinion), see [2]

This series adds a new "platform-roothub". This can be configured through
devicetree by passing a child-node with "reg = <0>" to the USB
controller. Additionally there has to be a child-node for each port on
the root-hub. Each of the child-nodes takes a "phys" and "phy-names"
property. This allows modeling the root-hub in devicetree similar to the
USB device binding (documented in devicetree/bindings/usb/usb-device.txt)
This avoids and backwards-compatibility problems (which was a concern
regardless of the solution, see [3]) since the binding for the root-hub
was previously not specified (and we're not using the "phys" property of
the controller, which might have served different purposes before,
depending on the drivers).

Additionally this integrates the new platform-roothub into xhci-plat.c
which automatically enables it for the dwc3 driver (in host-mode).


[0] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001945.html
[1] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001947.html
[2] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001818.html
[3] http://lists.infradead.org/pipermail/linux-amlogic/2017-January/001948.html

Martin Blumenstingl (2):
  usb: host: add a generic platform USB roothub driver
  usb: host: xhci: plat: integrate the platform-roothub

 .../devicetree/bindings/usb/usb-roothub.txt        |  41 ++++++
 Documentation/devicetree/bindings/usb/usb-xhci.txt |   7 +
 drivers/usb/host/Kconfig                           |   4 +
 drivers/usb/host/Makefile                          |   2 +
 drivers/usb/host/platform-roothub.c                | 146 +++++++++++++++++++++
 drivers/usb/host/platform-roothub.h                |  14 ++
 drivers/usb/host/xhci-plat.c                       |  37 +++++-
 drivers/usb/host/xhci.h                            |   3 +
 8 files changed, 251 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
 create mode 100644 drivers/usb/host/platform-roothub.c
 create mode 100644 drivers/usb/host/platform-roothub.h

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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

* [RFC PATCH 1/2] usb: host: add a generic platform USB roothub driver
From: Martin Blumenstingl @ 2017-01-11 15:29 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, mathias.nyman-ral2JQCrhuEAvxtiuMwx3w,
	vegard.nossum-QHcLZuEGTsvQT0dZR+AlfA,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
	treding-DDmLM1+adcrQT0dZR+AlfA, geert-Td1EMuHUCqxL1ZNQvxDV9g,
	manjunath.goudar-QSEj5FYQhm4dnm+yROfE0A, arnd-r2nGTMty4D4,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, ahaslam-rdvid1DuHRBWk0Htik3J/w,
	Martin Blumenstingl
In-Reply-To: <20170111152947.27088-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

Many SoC platforms have separate devices for the USB PHY which are
registered through the generic PHY framework. These PHYs have to be
enabled to make the USB controller actually work. They also have to be
disabled again on shutdown/suspend.

Currently (at least) the following HCI platform drivers are using custom
code to obtain all PHYs via devicetree for the roothub/controller and
disable/enable them when required:
- ehci-platform.c has ehci_platform_power_{on,off}
- xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
- ohci-platform.c has ohci_platform_power_{on,off}

These drivers are not using the generic devicetree USB device bindings
yet which were only introduced recently (documentation is available in
devicetree/bindings/usb/usb-device.txt).
With this new driver the usb2-phy and usb3-phy can be specified directly
in the child-node of the corresponding port of the roothub via
devicetree. This can be extended by not just parsing PHYs (some of the
other drivers listed above are for example also parsing a list of clocks
as well) when required.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 .../devicetree/bindings/usb/usb-roothub.txt        |  41 ++++++
 drivers/usb/host/Kconfig                           |   3 +
 drivers/usb/host/Makefile                          |   2 +
 drivers/usb/host/platform-roothub.c                | 146 +++++++++++++++++++++
 drivers/usb/host/platform-roothub.h                |  14 ++
 5 files changed, 206 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
 create mode 100644 drivers/usb/host/platform-roothub.c
 create mode 100644 drivers/usb/host/platform-roothub.h

diff --git a/Documentation/devicetree/bindings/usb/usb-roothub.txt b/Documentation/devicetree/bindings/usb/usb-roothub.txt
new file mode 100644
index 000000000000..96e152d3901c
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-roothub.txt
@@ -0,0 +1,41 @@
+Generic USB root-hub Properties
+
+similar to the USB device bindings (documented in usb-device.txt from the
+current directory) this provides support for configuring the root-hub.
+
+Required properties:
+- compatible: should be at least one of "usb1d6b,3", "usb1d6b,2"
+- reg: must be 0.
+- address-cells: must be 1
+- size-cells: must be 0
+- a sub-node per port supports the following properties:
+  - reg: the port number on the root-hub (mandatory)
+  - phys: optional, from the *Generic PHY* bindings (mandatory needed when
+    phy-names is given)
+  - phy-names: optional, from the *Generic PHY* bindings; supported names
+    are "usb2-phy" or "usb3-phy"
+
+Example:
+	&usb1 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		roothub@0 {
+			compatible = "usb1d6b,3", "usb1d6b,2";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0>;
+
+			port@1 {
+				reg = <1>;
+				usb-phy = <&usb2_phy1>, <&usb3_phy1>;
+				phy-names = "usb2-phy", "usb3-phy";
+			};
+
+			port@2 {
+				reg = <2>;
+				usb-phy = <&usb2_phy2>, <&usb3_phy2>;
+				phy-names = "usb2-phy", "usb3-phy";
+			};
+		};
+	}
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 6361fc739306..eb7009460322 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -797,6 +797,9 @@ config USB_HCD_SSB
 
 	  If unsure, say N.
 
+config USB_PLATFORM_ROOTHUB
+	bool
+
 config USB_HCD_TEST_MODE
 	bool "HCD test mode support"
 	---help---
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 2644537b7bcf..1f1e19c4a826 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -31,6 +31,8 @@ ifneq ($(CONFIG_USB), )
 	obj-$(CONFIG_PCI)	+= pci-quirks.o
 endif
 
+obj-$(CONFIG_USB_PLATFORM_ROOTHUB)	+= platform-roothub.o
+
 obj-$(CONFIG_USB_EHCI_HCD)	+= ehci-hcd.o
 obj-$(CONFIG_USB_EHCI_PCI)	+= ehci-pci.o
 obj-$(CONFIG_USB_EHCI_HCD_PLATFORM)	+= ehci-platform.o
diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c
new file mode 100644
index 000000000000..84837e42b006
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.c
@@ -0,0 +1,146 @@
+/*
+ * platform roothub driver - a virtual PHY device which passes all phy_*
+ * function calls to multiple (actual) PHY devices. This is comes handy when
+ * initializing all PHYs on a root-hub (to keep them all in the same state).
+ *
+ * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/phy/phy.h>
+#include <linux/of.h>
+#include <linux/usb/of.h>
+
+#include "platform-roothub.h"
+
+#define ROOTHUB_PORTNUM		0
+
+struct platform_roothub {
+	struct phy		*phy;
+	struct list_head	list;
+};
+
+static struct platform_roothub *platform_roothub_alloc(struct device *dev)
+{
+	struct platform_roothub *roothub_entry;
+
+	roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL);
+	if (!roothub_entry)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_LIST_HEAD(&roothub_entry->list);
+
+	return roothub_entry;
+}
+
+static int platform_roothub_add_phy(struct device *dev,
+				    struct device_node *port_np,
+				    const char *con_id, struct list_head *list)
+{
+	struct platform_roothub *roothub_entry;
+	struct phy *phy = devm_of_phy_get(dev, port_np, con_id);
+
+	if (IS_ERR_OR_NULL(phy)) {
+		if (!phy || PTR_ERR(phy) == -ENODEV)
+			return 0;
+		else
+			return PTR_ERR(phy);
+	}
+
+	roothub_entry = platform_roothub_alloc(dev);
+	if (IS_ERR(roothub_entry))
+		return PTR_ERR(roothub_entry);
+
+	roothub_entry->phy = phy;
+
+	list_add_tail(&roothub_entry->list, list);
+
+	return 0;
+}
+
+struct platform_roothub *platform_roothub_init(struct device *dev)
+{
+	struct device_node *roothub_np, *port_np;
+	struct platform_roothub *plat_roothub;
+	int err;
+
+	roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM);
+	if (!of_device_is_available(roothub_np))
+		return NULL;
+
+	plat_roothub = platform_roothub_alloc(dev);
+	if (IS_ERR(plat_roothub))
+		return plat_roothub;
+
+	for_each_available_child_of_node(roothub_np, port_np) {
+		err = platform_roothub_add_phy(dev, port_np, "usb2-phy",
+					       &plat_roothub->list);
+		if (err)
+			return ERR_PTR(err);
+
+		err = platform_roothub_add_phy(dev, port_np, "usb3-phy",
+					       &plat_roothub->list);
+		if (err)
+			return ERR_PTR(err);
+	}
+
+	return plat_roothub;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_init);
+
+int platform_roothub_power_on(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+	struct list_head *head;
+	int err;
+
+	if (!plat_roothub)
+		return 0;
+
+	head = &plat_roothub->list;
+
+	list_for_each_entry(roothub_entry, head, list) {
+		err = phy_init(roothub_entry->phy);
+		if (err)
+			goto err_out;
+
+		err = phy_power_on(roothub_entry->phy);
+		if (err) {
+			phy_exit(roothub_entry->phy);
+			goto err_out;
+		}
+	}
+
+	return 0;
+
+err_out:
+	list_for_each_entry_continue_reverse(roothub_entry, head, list) {
+		phy_power_off(roothub_entry->phy);
+		phy_exit(roothub_entry->phy);
+	}
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(platform_roothub_power_on);
+
+void platform_roothub_power_off(struct platform_roothub *plat_roothub)
+{
+	struct platform_roothub *roothub_entry;
+
+	if (!plat_roothub)
+		return;
+
+	list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) {
+		phy_power_off(roothub_entry->phy);
+		phy_exit(roothub_entry->phy);
+	}
+}
+EXPORT_SYMBOL_GPL(platform_roothub_power_off);
diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h
new file mode 100644
index 000000000000..bde0bf299e3b
--- /dev/null
+++ b/drivers/usb/host/platform-roothub.h
@@ -0,0 +1,14 @@
+#ifndef USB_HOST_PLATFORM_ROOTHUB_H
+#define USB_HOST_PLATFORM_ROOTHUB_H
+
+struct phy;
+struct device_node;
+
+struct platform_roothub;
+
+struct platform_roothub *platform_roothub_init(struct device *dev);
+
+int platform_roothub_power_on(struct platform_roothub *plat_roothub);
+void platform_roothub_power_off(struct platform_roothub *plat_roothub);
+
+#endif /* USB_HOST_PLATFORM_ROOTHUB_H */
-- 
2.11.0

--
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

* [RFC PATCH 2/2] usb: host: xhci: plat: integrate the platform-roothub
From: Martin Blumenstingl @ 2017-01-11 15:29 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	felipe.balbi-VuQAYsv1563Yd54FQh9/CA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A
  Cc: mark.rutland-5wv7dgnIgG8, mathias.nyman-ral2JQCrhuEAvxtiuMwx3w,
	vegard.nossum-QHcLZuEGTsvQT0dZR+AlfA,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
	treding-DDmLM1+adcrQT0dZR+AlfA, geert-Td1EMuHUCqxL1ZNQvxDV9g,
	manjunath.goudar-QSEj5FYQhm4dnm+yROfE0A, arnd-r2nGTMty4D4,
	tony-4v6yS6AI5VpBDgjK7y7TUQ, ahaslam-rdvid1DuHRBWk0Htik3J/w,
	Martin Blumenstingl
In-Reply-To: <20170111152947.27088-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>

This enables the platform-roothub for the xhci-plat driver. This allows
specifying a PHY for each port via devicetree. All PHYs will then be
enabled/disabled by the platform-roothub driver.

One example where this is required is the Amlogic GXL and GXM SoCs:
They are using a dwc3 USB controller with up to three ports enabled on
the internal roothub. Using only the top-level "phy" properties does not
work here since one can only specify one "usb2-phy" and one "usb3-phy",
while actually at least two "usb2-phy" have to be specified.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/usb-xhci.txt |  7 ++++
 drivers/usb/host/Kconfig                           |  1 +
 drivers/usb/host/xhci-plat.c                       | 37 ++++++++++++++++++++--
 drivers/usb/host/xhci.h                            |  3 ++
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index 0b7d8576001c..f05b2306ae19 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -28,6 +28,13 @@ Optional properties:
   - clocks: reference to a clock
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
 
+sub-nodes:
+- optionally there can be a node for the root-hub, see usb-roothub.txt in the
+  current directory
+- one or more nodes with reg 1-31 for each port to which a device is connected.
+  See usb-device.txt in the current directory for more information.
+
+
 Example:
 	usb@f0931000 {
 		compatible = "generic-xhci";
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index eb7009460322..b83b5aa89b76 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -36,6 +36,7 @@ config USB_XHCI_PCI
 config USB_XHCI_PLATFORM
 	tristate "Generic xHCI driver for a platform device"
 	select USB_XHCI_RCAR if ARCH_RENESAS
+	select USB_PLATFORM_ROOTHUB
 	---help---
 	  Adds an xHCI host driver for a generic platform device, which
 	  provides a memory space and an irq.
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index e94bfd07e6c3..a0211cebac71 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -248,6 +248,12 @@ static int xhci_plat_probe(struct platform_device *pdev)
 		goto disable_clk;
 	}
 
+	xhci->platform_roothub = platform_roothub_init(sysdev);
+	if (IS_ERR(xhci->platform_roothub)) {
+		ret = PTR_ERR(xhci->platform_roothub);
+		goto disable_clk;
+	}
+
 	if (device_property_read_bool(sysdev, "usb3-lpm-capable"))
 		xhci->quirks |= XHCI_LPM_SUPPORT;
 
@@ -266,10 +272,14 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto put_usb3_hcd;
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	ret = platform_roothub_power_on(xhci->platform_roothub);
 	if (ret)
 		goto disable_usb_phy;
 
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret)
+		goto disable_platform_roothub;
+
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
 	if (ret)
 		goto dealloc_usb2_hcd;
@@ -280,6 +290,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
 dealloc_usb2_hcd:
 	usb_remove_hcd(hcd);
 
+disable_platform_roothub:
+	platform_roothub_power_off(xhci->platform_roothub);
+
 disable_usb_phy:
 	usb_phy_shutdown(hcd->usb_phy);
 
@@ -305,6 +318,8 @@ static int xhci_plat_remove(struct platform_device *dev)
 	usb_remove_hcd(xhci->shared_hcd);
 	usb_phy_shutdown(hcd->usb_phy);
 
+	platform_roothub_power_off(xhci->platform_roothub);
+
 	usb_remove_hcd(hcd);
 	usb_put_hcd(xhci->shared_hcd);
 
@@ -320,6 +335,7 @@ static int xhci_plat_suspend(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
 
 	/*
 	 * xhci_suspend() needs `do_wakeup` to know whether host is allowed
@@ -329,15 +345,30 @@ static int xhci_plat_suspend(struct device *dev)
 	 * reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
 	 * also applies to runtime suspend.
 	 */
-	return xhci_suspend(xhci, device_may_wakeup(dev));
+	ret = xhci_suspend(xhci, device_may_wakeup(dev));
+	if (ret)
+		return ret;
+
+	platform_roothub_power_off(xhci->platform_roothub);
+
+	return 0;
 }
 
 static int xhci_plat_resume(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
 
-	return xhci_resume(xhci, 0);
+	ret = platform_roothub_power_on(xhci->platform_roothub);
+	if (ret)
+		return ret;
+
+	ret = xhci_resume(xhci, 0);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 2d7b6374b58d..c1d384223419 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -34,6 +34,8 @@
 #include	"xhci-ext-caps.h"
 #include "pci-quirks.h"
 
+#include "platform-roothub.h"
+
 /* xHCI PCI Configuration Registers */
 #define XHCI_SBRN_OFFSET	(0x60)
 
@@ -1559,6 +1561,7 @@ struct xhci_hcd {
 	struct msix_entry	*msix_entries;
 	/* optional clock */
 	struct clk		*clk;
+	struct platform_roothub	*platform_roothub;
 	/* data structures */
 	struct xhci_device_context_array *dcbaa;
 	struct xhci_ring	*cmd_ring;
-- 
2.11.0

--
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 1/5] pinctrl: core: Use delayed work for hogs
From: Linus Walleij @ 2017-01-11 15:33 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Geert Uytterhoeven, Haojian Zhuang, Masahiro Yamada,
	Grygorii Strashko, Nishanth Menon, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-omap@vger.kernel.org, Linux-Renesas
In-Reply-To: <20170110191908.GV2630@atomide.com>

On Tue, Jan 10, 2017 at 8:19 PM, Tony Lindgren <tony@atomide.com> wrote:

> Below is an experimental fix to intorduce pinctrl_start() that I've
> tested with pinctrl-single. Then we should probably make all pin controller
> drivers call pinctrl_start() to properly fix the issue of struct pinctrl_dev
> handle not being initialized before driver functions are called.

Hm I guess that could work, but can we keep pinctrl_register() with the old
semantics and add a separate pinctrl_register_and_defer()
for those who just wanna start it later by a separate call?

Then we don't need any special flags.

> Or do you guys have any better ideas?

Not really. So you mean revert the previous patch and apply something
like this instead?

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v2] gpio: pca953x: Add optional reset gpio control
From: Linus Walleij @ 2017-01-11 15:36 UTC (permalink / raw)
  To: Steve Longerbeam
  Cc: Alexandre Courbot, Rob Herring, Mark Rutland,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Steve Longerbeam
In-Reply-To: <1484076591-20834-2-git-send-email-steve_longerbeam@mentor.com>

On Tue, Jan 10, 2017 at 8:29 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:

> Add optional reset-gpios pin control. If present, de-assert the
> specified reset gpio pin to bring the chip out of reset.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org

v2 patch applied adding a few review tags.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2017-01-11 15:39 UTC (permalink / raw)
  To: M'boumba Cedric Madianga
  Cc: devicetree, Alexandre Torgue, Wolfram Sang, linux-kernel,
	Linus Walleij, Patrice Chotard, Russell King, Rob Herring,
	linux-i2c, Maxime Coquelin, linux-arm-kernel
In-Reply-To: <CAOAejn2eOy2sn1VkE979ne23Sj9L6+kaQDNpL1EUKb2m=6sGXw@mail.gmail.com>

On Wed, Jan 11, 2017 at 02:58:44PM +0100, M'boumba Cedric Madianga wrote:
> Hi Uwe,
> 
> 2017-01-11 9:22 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > Hello Cedric,
> >
> > On Thu, Jan 05, 2017 at 10:07:23AM +0100, M'boumba Cedric Madianga wrote:
> >> +/*
> >> + * In standard mode:
> >> + * SCL period = SCL high period = SCL low period = CCR * I2C parent clk period
> >> + *
> >> + * In fast mode:
> >> + * If Duty = 0; SCL high period = 1  * CCR * I2C parent clk period
                                         ^^
> >> + *           SCL low period  = 2  * CCR * I2C parent clk period
                                      ^^
> >> + * If Duty = 1; SCL high period = 9  * CCR * I2C parent clk period
                                         ^^
> >> + *           SCL low period  = 16 * CCR * I2C parent clk period

> > s/  \*/ */ several times
> 
> Sorry but I don't see where is the issue as the style for multi-line
> comments seems ok.
> Could you please clarify that point if possible ? Thanks in advance

There are several places with double spaces before * marked above.

> >> + * In order to reach 400 kHz with lower I2C parent clk frequencies we always set
> >> + * Duty = 1
> >> + *
> >> + * For both modes, we have CCR = SCL period * I2C parent clk frequency
> >> + * with scl_period = 5 microseconds in Standard mode and scl_period = 1
> > s/mode/Mode/
> 
> ok thanks
> 
> >
> >> + * microsecond in Fast Mode in order to satisfy scl_high and scl_low periods
> >> + * constraints defined by i2c bus specification
> >
> > I don't understand scl_period = 1 µs for Fast Mode. For a bus freqency
> > of 400 kHz we need low + high = 2.5 µs. Is there a factor 10 missing
> > somewhere?
> 
> As CCR = SCL_period * I2C parent clk frequency with minimal freq =
> 2Mhz and SCL_period = 1 we have:
> CCR = 1 * 2Mhz = 2.
> But to compute, scl_low and scl_high in Fast mode, we have to do the
> following thing as Duty=1:
> scl_high = 9 * CCR * I2C parent clk period
> scl_low = 16 * CCR * I2C parent clk period
> In our example:
> scl_high = 9 * 2 * 0,0000005 = 0,000009 sec = 9 µs
> scl_low = 16 * 2 * 0.0000005 = 0,000016 sec = 16 µs
> So low + high = 27 µs > 2,5 µs

For me 9 µs + 16 µs is 25 µs, resulting in 40 kHz. That's why I wondered
if there is a factor 10 missing somewhere.

> >> + */
> >> +static struct stm32f4_i2c_timings i2c_timings[] = {
> >> [...]
> >> +
> >> +/**
> >> + * stm32f4_i2c_hw_config() - Prepare I2C block
> >> + * @i2c_dev: Controller's private data
> >> + */
> >> +static int stm32f4_i2c_hw_config(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +     int ret = 0;
> >> +
> >> +     /* Disable I2C */
> >> +     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_PE);
> >> +
> >> +     ret = stm32f4_i2c_set_periph_clk_freq(i2c_dev);
> >> +     if (ret)
> >> +             return ret;
> >> +
> >> +     stm32f4_i2c_set_rise_time(i2c_dev);
> >> +
> >> +     stm32f4_i2c_set_speed_mode(i2c_dev);
> >> +
> >> +     stm32f4_i2c_set_filter(i2c_dev);
> >> +
> >> +     /* Enable I2C */
> >> +     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_PE);
> >
> > This function is called after a hw reset, so there should be no need to
> > use clr_bits and set_bits because the value read from hw should be
> > known.
> 
> ok thanks
> 
> >
> >> +     return ret;
> >
> > return 0;
> 
> ok thanks
> 
> >
> >> +}
> >> +
> >> +static int stm32f4_i2c_wait_free_bus(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >> +     u32 status;
> >> +     int ret;
> >> +
> >> +     ret = readl_relaxed_poll_timeout(i2c_dev->base + STM32F4_I2C_SR2,
> >> +                                      status,
> >> +                                      !(status & STM32F4_I2C_SR2_BUSY),
> >> +                                      10, 1000);
> >> +     if (ret) {
> >> +             dev_dbg(i2c_dev->dev, "bus not free\n");
> >> +             ret = -EBUSY;
> >> +     }
> >> +
> >> +     return ret;
> >> +}
> >> +
> >> +/**
> >> + * stm32f4_i2c_write_ byte() - Write a byte in the data register
> >> + * @i2c_dev: Controller's private data
> >> + * @byte: Data to write in the register
> >> + */
> >> +static void stm32f4_i2c_write_byte(struct stm32f4_i2c_dev *i2c_dev, u8 byte)
> >> +{
> >> +     writel_relaxed(byte, i2c_dev->base + STM32F4_I2C_DR);
> >> +}
> >> +
> >> +/**
> >> + * stm32f4_i2c_write_msg() - Fill the data register in write mode
> >> + * @i2c_dev: Controller's private data
> >> + *
> >> + * This function fills the data register with I2C transfer buffer
> >> + */
> >> +static void stm32f4_i2c_write_msg(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> >> +
> >> +     stm32f4_i2c_write_byte(i2c_dev, *msg->buf++);
> >> +     msg->count--;
> >> +}
> >> +
> >> +static void stm32f4_i2c_read_msg(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> >> +     u32 rbuf;
> >> +
> >> +     rbuf = readl_relaxed(i2c_dev->base + STM32F4_I2C_DR);
> >> +     *msg->buf++ = rbuf & 0xff;
> >
> > This is unnecessary. buf has an 8 bit wide type so
> >
> >         *msg->buf++ = rbuf;
> >
> > has the same effect. (ISTR this is something I already pointed out
> > earlier?)
> 
> Yes you are right.
> 
> >
> >> +     msg->count--;
> >> +}
> >> +
> >> +static void stm32f4_i2c_terminate_xfer(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> >> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> >> +
> >> +     stm32f4_i2c_disable_irq(i2c_dev);
> >> +
> >> +     reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +     if (msg->stop)
> >> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> >> +     else
> >> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> >> +
> >> +     complete(&i2c_dev->complete);
> >> +}
> >> +
> >> +/**
> >> + * stm32f4_i2c_handle_write() - Handle FIFO empty interrupt in case of write
> >> + * @i2c_dev: Controller's private data
> >> + */
> >> +static void stm32f4_i2c_handle_write(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> >> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> >> +
> >> +     if (msg->count) {
> >> +             stm32f4_i2c_write_msg(i2c_dev);
> >> +             if (!msg->count) {
> >> +                     /* Disable buffer interrupts for RXNE/TXE events */
> >> +                     stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> >> +             }
> >> +     } else {
> >> +             stm32f4_i2c_terminate_xfer(i2c_dev);
> >
> > Is stm32f4_i2c_terminate_xfer also called when arbitration is lost? If
> > yes, is it then right to set STM32F4_I2C_CR1_STOP or
> > STM32F4_I2C_CR1_START?
> 
> If arbitration is lost, stm32f4_i2c_terminate_xfer() is not called.
> In that case, we return -EAGAIN and i2c-core will retry by calling
> stm32f4_i2c_xfer()
> 
> >
> >> +     }
> >> +}
> >> +
> >> +/**
> >> + * stm32f4_i2c_handle_read() - Handle FIFO empty interrupt in case of read
> >> + * @i2c_dev: Controller's private data
> >> + */
> >> +static void stm32f4_i2c_handle_read(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> >> +     void __iomem *reg = i2c_dev->base + STM32F4_I2C_CR2;
> >> +
> >> +     switch (msg->count) {
> >> +     case 1:
> >> +             stm32f4_i2c_disable_irq(i2c_dev);
> >> +             stm32f4_i2c_read_msg(i2c_dev);
> >> +             complete(&i2c_dev->complete);
> >> +             break;
> >> +     /*
> >> +      * For 2 or 3-byte reception, we do not have to read the data register
> >> +      * when RXNE occurs as we have to wait for byte transferred finished
> >
> > it's hard to understand because if you don't know the hardware the
> > meaning of RXNE is unknown.
> 
> Ok I will replace RXNE by RX not empty in that comment
> 
> >
> >> +      * event before reading data. So, here we just disable buffer
> >> +      * interrupt in order to avoid another system preemption due to RXNE
> >> +      * event
> >> +      */
> >> +     case 2:
> >> +     case 3:
> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR2_ITBUFEN);
> >> +             break;
> >> +     /* For N byte reception with N > 3 we directly read data register */
> >> +     default:
> >> +             stm32f4_i2c_read_msg(i2c_dev);
> >> +     }
> >> +}
> >> +
> >> +/**
> >> + * stm32f4_i2c_handle_rx_btf() - Handle byte transfer finished interrupt
> >> + * in case of read
> >> + * @i2c_dev: Controller's private data
> >> + */
> >> +static void stm32f4_i2c_handle_rx_btf(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >
> > btf is a hw-related name. Maybe better use _done which is easier to
> > understand?
> 
> OK
> 
> >
> >> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> >> +     void __iomem *reg;
> >> +     u32 mask;
> >> +     int i;
> >> +
> >> +     switch (msg->count) {
> >> +     case 2:
> >> +             /*
> >> +              * In order to correctly send the Stop or Repeated Start
> >> +              * condition on the I2C bus, the STOP/START bit has to be set
> >> +              * before reading the last two bytes.
> >> +              * After that, we could read the last two bytes, disable
> >> +              * remaining interrupts and notify the end of xfer to the
> >> +              * client
> >
> > This is surprising. I didn't recheck the manual, but that looks very
> > uncomfortable.
> 
> I agree but this exactly the hardware way of working described in the
> reference manual.

IMHO that's a hw bug. This makes it for example impossible to implement
SMBus block transfers (I think).

> > How does this work, when I only want to read a single
> > byte? Same problem for ACK below.
> 
> For a single reception, we enable NACK and STOP or Repeatead START
> bits during address match.
> The NACK and STOP/START pulses are sent as soon as the data is
> received in the shift register.
> Please note that in that case, we don't have to wait BTF event to read the data.
> Data is read as soon as RXNE event occurs.
> 
> >
> >> +              */
> >> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +             if (msg->stop)
> >> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> >> +             else
> >> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> >> +
> >> +             for (i = 2; i > 0; i--)
> >> +                     stm32f4_i2c_read_msg(i2c_dev);
> >> +
> >> +             reg = i2c_dev->base + STM32F4_I2C_CR2;
> >> +             mask = STM32F4_I2C_CR2_ITEVTEN | STM32F4_I2C_CR2_ITERREN;
> >> +             stm32f4_i2c_clr_bits(reg, mask);
> >> +
> >> +             complete(&i2c_dev->complete);
> >> +             break;
> >> +     case 3:
> >> +             /*
> >> +              * In order to correctly send the ACK on the I2C bus for the
> >> +              * last two bytes, we have to set ACK bit before reading the
> >> +              * third last data byte
> >> +              */
> >> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> >> +             stm32f4_i2c_read_msg(i2c_dev);
> >> +             break;
> >> +     default:
> >> +             stm32f4_i2c_read_msg(i2c_dev);
> >> +     }
> >> +}
> >> +
> >> +/**
> >> + * stm32f4_i2c_handle_rx_addr() - Handle address matched interrupt in case of
> >> + * master receiver
> >> + * @i2c_dev: Controller's private data
> >> + */
> >> +static void stm32f4_i2c_handle_rx_addr(struct stm32f4_i2c_dev *i2c_dev)
> >> +{
> >> +     struct stm32f4_i2c_msg *msg = &i2c_dev->msg;
> >> +     void __iomem *reg;
> >> +
> >> +     switch (msg->count) {
> >> +     case 0:
> >> +             stm32f4_i2c_terminate_xfer(i2c_dev);
> >> +             /* Clear ADDR flag */
> >> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> >> +             break;
> >> +     case 1:
> >> +             /*
> >> +              * Single byte reception:
> >
> > This also happens for the last byte of a 5 byte transfer, right?
> 
> For a 5 byte transfer the behavior is different:
> We have to read data from DR (data register)  as soon as the RXNE (RX
> not empty) event occurs for data1, data2 and data3 (until N-2 data for
> a more generic case)
> The ACK is automatically sent as soon as the data is received in the
> shift register as the I2C controller was configured to do that during
> adress match phase.
> 
> For data3 (N-2 data), we wait for BTF (Byte Transfer finished) event
> in order to set NACK before reading DR.
> This event occurs when a new data has been received in shift register
> (in our case data4 or N-1 data) but the prevoius data in DR (in our
> case data3 or N-2 data) has not been read yet.
> In that way, the NACK pulse will be correctly generated after the last
> received data byte.
> 
> For data4 and data5, we wait for BTF event (data4 or N-1 data in DR
> and data5 or N data in shift register), set STOP or repeated Start in
> order to correctly sent the right pulse after the last received data
> byte and run 2 consecutives read of DR.

So "Single byte reception" above is wrong, as this case is also used for
longer transfers and should be updated accordingly.

> >> +              * Enable NACK, clear ADDR flag and generate STOP or RepSTART
> >> +              */
> >> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> >> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> >> +             if (msg->stop)
> >> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_STOP);
> >> +             else
> >> +                     stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_START);
> >> +             break;
> >> +     case 2:
> >> +             /*
> >> +              * 2-byte reception:
> >> +              * Enable NACK and set POS
> >
> > What is POS?
> POS is used to define the position of the (N)ACK pulse
> 0: ACK is generated when the current is being received in the shift register
> 1: ACK is generated when the next byte which will be received in the
> shift register (used for 2-byte reception)

Can you please put this into the comment. "POS" isn't much helpful
there.

> 
> >
> >> +              */
> >> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> >> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
> >
> > You could get rid of this, when caching the value of CR1. Would save two
> > register reads here. This doesn't work for all registers, but it should
> > be possible to apply for most of them, maybe enough to get rid of the
> > clr_bits and set_bits function.
> >
> >> +             readl_relaxed(i2c_dev->base + STM32F4_I2C_SR2);
> >> +             break;
> >> +
> >> +     default:
> >> +             /* N-byte reception: Enable ACK */
> >> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_ACK);
> >
> > Do you need to set ACK for each byte transferred?
> I need to do that in order to be SMBus compatible and the ACK/NACK
> seems to be used by default in Documentation/i2c/i2c-protocol file.

Yeah, protocol wise you need to ack each byte. I just wondered if you
need to set the hardware bit for each byte or if it is retained in
hardware until unset by a register write.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* Re: [PATCH v8 2/5] i2c: Add STM32F4 I2C driver
From: Uwe Kleine-König @ 2017-01-11 15:42 UTC (permalink / raw)
  To: M'boumba Cedric Madianga
  Cc: Wolfram Sang, Rob Herring, Maxime Coquelin, Alexandre Torgue,
	Linus Walleij, Patrice Chotard, Russell King, linux-i2c,
	devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <CAOAejn2cDzPgqvWZf7ASvtST+aDeaAebf=1aMWA9Zd8CDg4pmA@mail.gmail.com>

Hello Cedric,

On Wed, Jan 11, 2017 at 03:20:41PM +0100, M'boumba Cedric Madianga wrote:
> >
> >> +              */
> >> +             reg = i2c_dev->base + STM32F4_I2C_CR1;
> >> +             stm32f4_i2c_clr_bits(reg, STM32F4_I2C_CR1_ACK);
> >> +             stm32f4_i2c_set_bits(reg, STM32F4_I2C_CR1_POS);
> >
> > You could get rid of this, when caching the value of CR1. Would save two
> > register reads here. This doesn't work for all registers, but it should
> > be possible to apply for most of them, maybe enough to get rid of the
> > clr_bits and set_bits function.
> 
> I agree at many places I could save registers read by not using
> clr_bits and set_bits function when the registers in question has been
> already read.
> But it is not enough to get rid of the clr_bits and set_bits function.
> For example when calling stm32f4_i2c_terminate_xfer(), the CR1
> register is never read before so set_bits function is useful.

I didn't double check the manual, but I would expect that CR1 isn't
modified by hardware. So you can cache the result in the driver data
structure and do the necessary modifications with that one.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH v3] ARM: dts: qcom: Add apq8064 CoreSight components
From: Georgi Djakov @ 2017-01-11 15:50 UTC (permalink / raw)
  To: andy.gross
  Cc: robh+dt, devicetree, mathieu.poirier, zhang.chunyan, iivanov.xz,
	linux-arm-msm, linux-kernel, linux-arm-kernel, georgi.djakov

From: "Ivan T. Ivanov" <ivan.ivanov@linaro.org>

Add initial set of CoreSight components found on Qualcomm apq8064 based
platforms, including the IFC6410 board.

Signed-off-by: Ivan T. Ivanov <ivan.ivanov@linaro.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---
Changes since v2 (https://lkml.org/lkml/2016/11/21/522)
 * Rebase to linux-next

Changes since v1 (https://lkml.org/lkml/2016/11/17/474)
 * Moved everything into the SoC dtsi file as suggested by Stephen Boyd.
 * Updated commit message.
 * Got Ack from Mathieu.

 arch/arm/boot/dts/qcom-apq8064.dtsi | 189 +++++++++++++++++++++++++++++++++++-
 1 file changed, 185 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 407a4610f4a7..4b77075ef731 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -28,7 +28,7 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		cpu@0 {
+		CPU0: cpu@0 {
 			compatible = "qcom,krait";
 			enable-method = "qcom,kpss-acc-v1";
 			device_type = "cpu";
@@ -39,7 +39,7 @@
 			cpu-idle-states = <&CPU_SPC>;
 		};
 
-		cpu@1 {
+		CPU1: cpu@1 {
 			compatible = "qcom,krait";
 			enable-method = "qcom,kpss-acc-v1";
 			device_type = "cpu";
@@ -50,7 +50,7 @@
 			cpu-idle-states = <&CPU_SPC>;
 		};
 
-		cpu@2 {
+		CPU2: cpu@2 {
 			compatible = "qcom,krait";
 			enable-method = "qcom,kpss-acc-v1";
 			device_type = "cpu";
@@ -61,7 +61,7 @@
 			cpu-idle-states = <&CPU_SPC>;
 		};
 
-		cpu@3 {
+		CPU3: cpu@3 {
 			compatible = "qcom,krait";
 			enable-method = "qcom,kpss-acc-v1";
 			device_type = "cpu";
@@ -1420,6 +1420,187 @@
 				};
 			};
 		};
+
+		etb@1a01000 {
+			compatible = "coresight-etb10", "arm,primecell";
+			reg = <0x1a01000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+
+			port {
+				etb_in: endpoint {
+					slave-mode;
+					remote-endpoint = <&replicator_out0>;
+				};
+			};
+		};
+
+		tpiu@1a03000 {
+			compatible = "arm,coresight-tpiu", "arm,primecell";
+			reg = <0x1a03000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+
+			port {
+				tpiu_in: endpoint {
+					slave-mode;
+					remote-endpoint = <&replicator_out1>;
+				};
+			};
+		};
+
+		replicator {
+			compatible = "arm,coresight-replicator";
+
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					replicator_out0: endpoint {
+						remote-endpoint = <&etb_in>;
+					};
+				};
+				port@1 {
+					reg = <1>;
+					replicator_out1: endpoint {
+						remote-endpoint = <&tpiu_in>;
+					};
+				};
+				port@2 {
+					reg = <0>;
+					replicator_in: endpoint {
+						slave-mode;
+						remote-endpoint = <&funnel_out>;
+					};
+				};
+			};
+		};
+
+		funnel@1a04000 {
+			compatible = "arm,coresight-funnel", "arm,primecell";
+			reg = <0x1a04000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				/*
+				 * Not described input ports:
+				 * 2 - connected to STM component
+				 * 3 - not-connected
+				 * 6 - not-connected
+				 * 7 - not-connected
+				 */
+				port@0 {
+					reg = <0>;
+					funnel_in0: endpoint {
+						slave-mode;
+						remote-endpoint = <&etm0_out>;
+					};
+				};
+				port@1 {
+					reg = <1>;
+					funnel_in1: endpoint {
+						slave-mode;
+						remote-endpoint = <&etm1_out>;
+					};
+				};
+				port@4 {
+					reg = <4>;
+					funnel_in4: endpoint {
+						slave-mode;
+						remote-endpoint = <&etm2_out>;
+					};
+				};
+				port@5 {
+					reg = <5>;
+					funnel_in5: endpoint {
+						slave-mode;
+						remote-endpoint = <&etm3_out>;
+					};
+				};
+				port@8 {
+					reg = <0>;
+					funnel_out: endpoint {
+						remote-endpoint = <&replicator_in>;
+					};
+				};
+			};
+		};
+
+		etm@1a1c000 {
+			compatible = "arm,coresight-etm3x", "arm,primecell";
+			reg = <0x1a1c000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+
+			cpu = <&CPU0>;
+
+			port {
+				etm0_out: endpoint {
+					remote-endpoint = <&funnel_in0>;
+				};
+			};
+		};
+
+		etm@1a1d000 {
+			compatible = "arm,coresight-etm3x", "arm,primecell";
+			reg = <0x1a1d000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+
+			cpu = <&CPU1>;
+
+			port {
+				etm1_out: endpoint {
+					remote-endpoint = <&funnel_in1>;
+				};
+			};
+		};
+
+		etm@1a1e000 {
+			compatible = "arm,coresight-etm3x", "arm,primecell";
+			reg = <0x1a1e000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+
+			cpu = <&CPU2>;
+
+			port {
+				etm2_out: endpoint {
+					remote-endpoint = <&funnel_in4>;
+				};
+			};
+		};
+
+		etm@1a1f000 {
+			compatible = "arm,coresight-etm3x", "arm,primecell";
+			reg = <0x1a1f000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>;
+			clock-names = "apb_pclk";
+
+			cpu = <&CPU3>;
+
+			port {
+				etm3_out: endpoint {
+					remote-endpoint = <&funnel_in5>;
+				};
+			};
+		};
 	};
 };
 #include "qcom-apq8064-pins.dtsi"

^ permalink raw reply related

* [RESEND PATCH v5] arm64: dts: qcom: Add msm8916 CoreSight components
From: Georgi Djakov @ 2017-01-11 15:59 UTC (permalink / raw)
  To: andy.gross
  Cc: robh+dt, devicetree, mathieu.poirier, zhang.chunyan, iivanov.xz,
	linux-arm-msm, linux-kernel, linux-arm-kernel, georgi.djakov

From: "Ivan T. Ivanov" <ivan.ivanov@linaro.org>

Add initial set of CoreSight components found on Qualcomm msm8916 and
apq8016 based platforms, including the DragonBoard 410c board.

Signed-off-by: Ivan T. Ivanov <ivan.ivanov@linaro.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
---

Changes since v4: (https://lkml.org/lkml/2016/11/17/533)
 * Move everything into the SoC dtsi file as suggested by Stephen Boyd.
 * Updated commit message.
 * Got Ack from Mathieu.

Changes since v3: (https://lkml.org/lkml/2015/5/11/134)
 * Include msm8916-coresight.dtsi into msm8916.dtsi

Changes since v2: (https://lkml.org/lkml/2015/4/29/242)
 * Added "1x" to "qcom,coresight-replicator" compatible string, to match what
   devicetree bindings documentations says.

 arch/arm64/boot/dts/qcom/msm8916.dtsi | 241 ++++++++++++++++++++++++++++++++++
 1 file changed, 241 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index f8ff327667c5..014d76e7dddf 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -14,6 +14,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/qcom,gcc-msm8916.h>
 #include <dt-bindings/reset/qcom,gcc-msm8916.h>
+#include <dt-bindings/clock/qcom,rpmcc.h>
 
 / {
 	model = "Qualcomm Technologies, Inc. MSM8916";
@@ -856,6 +857,246 @@
 				memory-region = <&mpss_mem>;
 			};
 		};
+
+		tpiu@820000 {
+			compatible = "arm,coresight-tpiu", "arm,primecell";
+			reg = <0x820000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			port {
+				tpiu_in: endpoint {
+					slave-mode;
+					remote-endpoint = <&replicator_out1>;
+				};
+			};
+		};
+
+		funnel@821000 {
+			compatible = "arm,coresight-funnel", "arm,primecell";
+			reg = <0x821000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				/*
+				 * Not described input ports:
+				 * 0 - connected to Resource and Power Manger CPU ETM
+				 * 1 - not-connected
+				 * 2 - connected to Modem CPU ETM
+				 * 3 - not-connected
+				 * 5 - not-connected
+				 * 6 - connected trought funnel to Wireless CPU ETM
+				 * 7 - connected to STM component
+				 */
+
+				port@4 {
+					reg = <4>;
+					funnel0_in4: endpoint {
+						slave-mode;
+						remote-endpoint = <&funnel1_out>;
+					};
+				};
+				port@8 {
+					reg = <0>;
+					funnel0_out: endpoint {
+						remote-endpoint = <&etf_in>;
+					};
+				};
+			};
+		};
+
+		replicator@824000 {
+			compatible = "qcom,coresight-replicator1x", "arm,primecell";
+			reg = <0x824000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					replicator_out0: endpoint {
+						remote-endpoint = <&etr_in>;
+					};
+				};
+				port@1 {
+					reg = <1>;
+					replicator_out1: endpoint {
+						remote-endpoint = <&tpiu_in>;
+					};
+				};
+				port@2 {
+					reg = <0>;
+					replicator_in: endpoint {
+						slave-mode;
+						remote-endpoint = <&etf_out>;
+					};
+				};
+			};
+		};
+
+		etf@825000 {
+			compatible = "arm,coresight-tmc", "arm,primecell";
+			reg = <0x825000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					etf_out: endpoint {
+						slave-mode;
+						remote-endpoint = <&funnel0_out>;
+					};
+				};
+				port@1 {
+					reg = <0>;
+					etf_in: endpoint {
+						remote-endpoint = <&replicator_in>;
+					};
+				};
+			};
+		};
+
+		etr@826000 {
+			compatible = "arm,coresight-tmc", "arm,primecell";
+			reg = <0x826000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			port {
+				etr_in: endpoint {
+					slave-mode;
+					remote-endpoint = <&replicator_out0>;
+				};
+			};
+		};
+
+		funnel@841000 {	/* APSS funnel only 4 inputs are used */
+			compatible = "arm,coresight-funnel", "arm,primecell";
+			reg = <0x841000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				port@0 {
+					reg = <0>;
+					funnel1_in0: endpoint {
+						slave-mode;
+						remote-endpoint = <&etm0_out>;
+					};
+				};
+				port@1 {
+					reg = <1>;
+					funnel1_in1: endpoint {
+						slave-mode;
+						remote-endpoint = <&etm1_out>;
+					};
+				};
+				port@2 {
+					reg = <2>;
+					funnel1_in2: endpoint {
+						slave-mode;
+						remote-endpoint = <&etm2_out>;
+					};
+				};
+				port@3 {
+					reg = <3>;
+					funnel1_in3: endpoint {
+						slave-mode;
+						remote-endpoint = <&etm3_out>;
+					};
+				};
+				port@4 {
+					reg = <0>;
+					funnel1_out: endpoint {
+						remote-endpoint = <&funnel0_in4>;
+					};
+				};
+			};
+		};
+
+		etm@85c000 {
+			compatible = "arm,coresight-etm4x", "arm,primecell";
+			reg = <0x85c000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			cpu = <&CPU0>;
+
+			port {
+				etm0_out: endpoint {
+				remote-endpoint = <&funnel1_in0>;
+				};
+			};
+		};
+
+		etm@85d000 {
+			compatible = "arm,coresight-etm4x", "arm,primecell";
+			reg = <0x85d000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			cpu = <&CPU1>;
+
+			port {
+				etm1_out: endpoint {
+					remote-endpoint = <&funnel1_in1>;
+				};
+			};
+		};
+
+		etm@85e000 {
+			compatible = "arm,coresight-etm4x", "arm,primecell";
+			reg = <0x85e000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			cpu = <&CPU2>;
+
+			port {
+				etm2_out: endpoint {
+					remote-endpoint = <&funnel1_in2>;
+				};
+			};
+		};
+
+		etm@85f000 {
+			compatible = "arm,coresight-etm4x", "arm,primecell";
+			reg = <0x85f000 0x1000>;
+
+			clocks = <&rpmcc RPM_QDSS_CLK>, <&rpmcc RPM_QDSS_A_CLK>;
+			clock-names = "apb_pclk", "atclk";
+
+			cpu = <&CPU3>;
+
+			port {
+				etm3_out: endpoint {
+					remote-endpoint = <&funnel1_in3>;
+				};
+			};
+		};
 	};
 
 	smd {

^ permalink raw reply related

* [PATCH 1/2] clk: vc5: Add bindings for IDT VersaClock 5P49V5923 and 5P49V5933
From: Marek Vasut @ 2017-01-11 16:16 UTC (permalink / raw)
  To: linux-clk
  Cc: Marek Vasut, Michael Turquette, Stephen Boyd, Laurent Pinchart,
	Rob Herring, devicetree, linux-renesas-soc

Add bindings for IDT VersaClock 5 5P49V5923 and 5P49V5933 chips.
These are I2C clock generators with optional clock source from
either XTal or dedicated clock generator and, depending on the
model, two or more clock outputs.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Cc: Rob Herring <robh@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
 .../devicetree/bindings/clock/idt,versaclock5.txt  | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/idt,versaclock5.txt

diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
new file mode 100644
index 000000000000..82ebed9a8a15
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt
@@ -0,0 +1,43 @@
+Binding for IDT VersaClock5 programmable i2c clock generator.
+
+The IDT VersaClock5 are programmable i2c clock generators providing
+from 3 to 12 output clocks.
+
+==I2C device node==
+
+Required properties:
+- compatible:	shall be one of "idt,5p49v5923" , "idt,5p49v5933".
+- reg:		i2c device address, shall be 0x68 or 0x6a.
+- #clock-cells:	from common clock binding; shall be set to 1.
+- clocks:	from common clock binding; list of parent clock handles,
+		- 5p49v5923: (required) either or both of XTAL or CLKIN
+					reference clock.
+		- 5p49v5933: (optional) property not present (internal
+					Xtal used) or CLKIN reference
+					clock.
+- clock-names:	from common clock binding; clock input names, can be
+		- 5p49v5923: (required) either or both of "xin", "clkin".
+		- 5p49v5933: (optional) property not present or "clkin".
+
+==Example==
+
+/* 25MHz reference crystal */
+ref25: ref25m {
+	compatible = "fixed-clock";
+	#clock-cells = <0>;
+	clock-frequency = <25000000>;
+};
+
+i2c-master-node {
+
+	/* IDT 5P49V5923 i2c clock generator */
+	vc5: clock-generator@6a {
+		compatible = "idt,5p49v5923";
+		reg = <0x6a>;
+		#clock-cells = <1>;
+
+		/* Connect XIN input to 25MHz reference */
+		clocks = <&ref25m>;
+		clock-names = "xin";
+	};
+};
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH v4 1/3] arm64: dts: exynos: add DECON_TV node
From: Krzysztof Kozlowski @ 2017-01-11 16:21 UTC (permalink / raw)
  To: Andrzej Hajda
  Cc: linux-samsung-soc, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz,
	Marek Szyprowski, Inki Dae, Rob Herring, Mark Rutland,
	Javier Martinez Canillas, devicetree, Andi Shyti, Chanwoo Choi
In-Reply-To: <1484123500-20488-1-git-send-email-a.hajda@samsung.com>

On Wed, Jan 11, 2017 at 09:31:38AM +0100, Andrzej Hajda wrote:
> DECON_TV is 2nd display controller on Exynos5433, used in HDMI path
> or 2nd DSI path.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> Tested-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
> v4:
>   - changed prefix
>   - rebased on '68cb4b3 Merge branch 'next/dt64' into for-next'
> ---
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi | 43 ++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)

Thanks, applied.

Best regards,
Krzysztof

^ permalink raw reply


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