| Message ID | 20210212164541.8986-1-uli+renesas@fpond.eu (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | i2c: rcar: implement atomic transfers | expand |
Hi Uli, > Implements atomic transfers. Tested by rebooting an r8a7790 Lager board > after connecting the i2c-rcar controller to the PMIC in > arch/arm/boot/dts/r8a7790-lager.dts like so: > > compatible = "i2c-demux-pinctrl"; > pinctrl-names = "default"; > pinctrl-0 = <&pmic_irq_pins>; > - i2c-parent = <&iic3>, <&i2c3>; > + i2c-parent = <&i2c3>, <&iic3>; > i2c-bus-name = "i2c-pwr"; > #address-cells = <1>; > #size-cells = <0>; > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> Doesn't apply against 5.11 or i2c/for-next. What is the base here? Regards, Wolfram
> On 02/18/2021 11:49 AM Wolfram Sang <wsa@the-dreams.de> wrote: > > Implements atomic transfers. Tested by rebooting an r8a7790 Lager board > > after connecting the i2c-rcar controller to the PMIC in > > arch/arm/boot/dts/r8a7790-lager.dts like so: > > > > compatible = "i2c-demux-pinctrl"; > > pinctrl-names = "default"; > > pinctrl-0 = <&pmic_irq_pins>; > > - i2c-parent = <&iic3>, <&i2c3>; > > + i2c-parent = <&i2c3>, <&iic3>; > > i2c-bus-name = "i2c-pwr"; > > #address-cells = <1>; > > #size-cells = <0>; > > > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> > > Doesn't apply against 5.11 or i2c/for-next. What is the base here? renesas-drivers-2021-02-09-v5.11-rc7 CU Uli
On Thu, Feb 18, 2021 at 05:03:41PM +0100, Ulrich Hecht wrote: > > > On 02/18/2021 11:49 AM Wolfram Sang <wsa@the-dreams.de> wrote: > > > Implements atomic transfers. Tested by rebooting an r8a7790 Lager board > > > after connecting the i2c-rcar controller to the PMIC in > > > arch/arm/boot/dts/r8a7790-lager.dts like so: > > > > > > compatible = "i2c-demux-pinctrl"; > > > pinctrl-names = "default"; > > > pinctrl-0 = <&pmic_irq_pins>; > > > - i2c-parent = <&iic3>, <&i2c3>; > > > + i2c-parent = <&i2c3>, <&iic3>; > > > i2c-bus-name = "i2c-pwr"; > > > #address-cells = <1>; > > > #size-cells = <0>; > > > > > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> > > > > Doesn't apply against 5.11 or i2c/for-next. What is the base here? > > renesas-drivers-2021-02-09-v5.11-rc7 Ok, now it applies, good. Testing went also fine, works nicely. From a maintaining perspective, I think, though, we should have only one master_xfer() function and use 'atomic_xfer' in there to skip stuff etc. Otherwise fixes in one function might be forgotten in the other one. Or?
> On 02/19/2021 1:01 PM Wolfram Sang <wsa@the-dreams.de> wrote: > From a maintaining perspective, I think, though, we should have only one > master_xfer() function and use 'atomic_xfer' in there to skip stuff etc. > Otherwise fixes in one function might be forgotten in the other one. Or? My guesstimate is that the amount of code required is about the same for both approaches, but having two separate implementations makes both of them a lot more readable. CU Uli
On Fri, Feb 12, 2021 at 5:45 PM Ulrich Hecht <uli+renesas@fpond.eu> wrote: > Implements atomic transfers. Tested by rebooting an r8a7790 Lager board > after connecting the i2c-rcar controller to the PMIC in > arch/arm/boot/dts/r8a7790-lager.dts like so: > > compatible = "i2c-demux-pinctrl"; > pinctrl-names = "default"; > pinctrl-0 = <&pmic_irq_pins>; > - i2c-parent = <&iic3>, <&i2c3>; > + i2c-parent = <&i2c3>, <&iic3>; > i2c-bus-name = "i2c-pwr"; > #address-cells = <1>; > #size-cells = <0>; > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> LGTM (IANIE = ... No I2C Expert), so Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On Fri, Feb 12, 2021 at 05:45:41PM +0100, Ulrich Hecht wrote: > Implements atomic transfers. Tested by rebooting an r8a7790 Lager board > after connecting the i2c-rcar controller to the PMIC in > arch/arm/boot/dts/r8a7790-lager.dts like so: > > compatible = "i2c-demux-pinctrl"; > pinctrl-names = "default"; > pinctrl-0 = <&pmic_irq_pins>; > - i2c-parent = <&iic3>, <&i2c3>; > + i2c-parent = <&i2c3>, <&iic3>; > i2c-bus-name = "i2c-pwr"; > #address-cells = <1>; > #size-cells = <0>; > > Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> Applied to for-next, thanks!
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c index 12f6d452c0f7..1f0c164f671a 100644 --- a/drivers/i2c/busses/i2c-rcar.c +++ b/drivers/i2c/busses/i2c-rcar.c @@ -141,6 +141,7 @@ struct rcar_i2c_priv { enum dma_data_direction dma_direction; struct reset_control *rstc; + bool atomic_xfer; int irq; struct i2c_client *host_notify_client; @@ -353,7 +354,9 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv) rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START); rcar_i2c_write(priv, ICMSR, 0); } - rcar_i2c_write(priv, ICMIER, read ? RCAR_IRQ_RECV : RCAR_IRQ_SEND); + + if (!priv->atomic_xfer) + rcar_i2c_write(priv, ICMIER, read ? RCAR_IRQ_RECV : RCAR_IRQ_SEND); } static void rcar_i2c_next_msg(struct rcar_i2c_priv *priv) @@ -418,7 +421,7 @@ static bool rcar_i2c_dma(struct rcar_i2c_priv *priv) int len; /* Do various checks to see if DMA is feasible at all */ - if (IS_ERR(chan) || msg->len < RCAR_MIN_DMA_LEN || + if (priv->atomic_xfer || IS_ERR(chan) || msg->len < RCAR_MIN_DMA_LEN || !(msg->flags & I2C_M_DMA_SAFE) || (read && priv->flags & ID_P_NO_RXDMA)) return false; @@ -646,7 +649,8 @@ static irqreturn_t rcar_i2c_irq(int irq, struct rcar_i2c_priv *priv, u32 msr) /* Nack */ if (msr & MNR) { /* HW automatically sends STOP after received NACK */ - rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP); + if (!priv->atomic_xfer) + rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP); priv->flags |= ID_NACK; goto out; } @@ -667,7 +671,8 @@ static irqreturn_t rcar_i2c_irq(int irq, struct rcar_i2c_priv *priv, u32 msr) if (priv->flags & ID_DONE) { rcar_i2c_write(priv, ICMIER, 0); rcar_i2c_write(priv, ICMSR, 0); - wake_up(&priv->wait); + if (!priv->atomic_xfer) + wake_up(&priv->wait); } return IRQ_HANDLED; @@ -684,7 +689,8 @@ static irqreturn_t rcar_i2c_gen2_irq(int irq, void *ptr) /* Only handle interrupts that are currently enabled */ msr = rcar_i2c_read(priv, ICMSR); - msr &= rcar_i2c_read(priv, ICMIER); + if (!priv->atomic_xfer) + msr &= rcar_i2c_read(priv, ICMIER); return rcar_i2c_irq(irq, priv, msr); } @@ -696,7 +702,8 @@ static irqreturn_t rcar_i2c_gen3_irq(int irq, void *ptr) /* Only handle interrupts that are currently enabled */ msr = rcar_i2c_read(priv, ICMSR); - msr &= rcar_i2c_read(priv, ICMIER); + if (!priv->atomic_xfer) + msr &= rcar_i2c_read(priv, ICMIER); /* * Clear START or STOP immediately, except for REPSTART after read or @@ -804,6 +811,8 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap, int i, ret; long time_left; + priv->atomic_xfer = false; + pm_runtime_get_sync(dev); /* Check bus state before init otherwise bus busy info will be lost */ @@ -858,6 +867,68 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap, return ret; } +static int rcar_i2c_master_xfer_atomic(struct i2c_adapter *adap, + struct i2c_msg *msgs, + int num) +{ + struct rcar_i2c_priv *priv = i2c_get_adapdata(adap); + struct device *dev = rcar_i2c_priv_to_dev(priv); + unsigned long j; + bool time_left; + int ret; + + priv->atomic_xfer = true; + + pm_runtime_get_sync(dev); + + /* Check bus state before init otherwise bus busy info will be lost */ + ret = rcar_i2c_bus_barrier(priv); + if (ret < 0) + goto out; + + rcar_i2c_init(priv); + + /* init first message */ + priv->msg = msgs; + priv->msgs_left = num; + priv->flags = (priv->flags & ID_P_MASK) | ID_FIRST_MSG; + rcar_i2c_prepare_msg(priv); + + j = jiffies + num * adap->timeout; + do { + u32 msr = rcar_i2c_read(priv, ICMSR); + + msr &= (rcar_i2c_is_recv(priv) ? RCAR_IRQ_RECV : RCAR_IRQ_SEND) | RCAR_IRQ_STOP; + + if (msr) { + if (priv->devtype < I2C_RCAR_GEN3) + rcar_i2c_gen2_irq(0, priv); + else + rcar_i2c_gen3_irq(0, priv); + } + + time_left = time_before_eq(jiffies, j); + } while (!(priv->flags & ID_DONE) && time_left); + + if (!time_left) { + rcar_i2c_init(priv); + ret = -ETIMEDOUT; + } else if (priv->flags & ID_NACK) { + ret = -ENXIO; + } else if (priv->flags & ID_ARBLOST) { + ret = -EAGAIN; + } else { + ret = num - priv->msgs_left; /* The number of transfer */ + } +out: + pm_runtime_put(dev); + + if (ret < 0 && ret != -ENXIO) + dev_err(dev, "error %d : %x\n", ret, priv->flags); + + return ret; +} + static int rcar_reg_slave(struct i2c_client *slave) { struct rcar_i2c_priv *priv = i2c_get_adapdata(slave->adapter); @@ -922,6 +993,7 @@ static u32 rcar_i2c_func(struct i2c_adapter *adap) static const struct i2c_algorithm rcar_i2c_algo = { .master_xfer = rcar_i2c_master_xfer, + .master_xfer_atomic = rcar_i2c_master_xfer_atomic, .functionality = rcar_i2c_func, .reg_slave = rcar_reg_slave, .unreg_slave = rcar_unreg_slave,
Implements atomic transfers. Tested by rebooting an r8a7790 Lager board after connecting the i2c-rcar controller to the PMIC in arch/arm/boot/dts/r8a7790-lager.dts like so: compatible = "i2c-demux-pinctrl"; pinctrl-names = "default"; pinctrl-0 = <&pmic_irq_pins>; - i2c-parent = <&iic3>, <&i2c3>; + i2c-parent = <&i2c3>, <&iic3>; i2c-bus-name = "i2c-pwr"; #address-cells = <1>; #size-cells = <0>; Signed-off-by: Ulrich Hecht <uli+renesas@fpond.eu> --- drivers/i2c/busses/i2c-rcar.c | 84 ++++++++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 6 deletions(-)