Re: [PATCH 02/14] dt-bindings: arm: don't embed SoC name into the ULCB boards' compatible
Hello Geert, Laurent, Morimoto-san, On Tue, Aug 07, 2018 at 10:30:14AM +0200, Geert Uytterhoeven wrote: > Hi Laurent, > > On Tue, Aug 7, 2018 at 10:21 AM Laurent Pinchart > wrote: > > On Tuesday, 7 August 2018 11:18:11 EEST Kuninori Morimoto wrote: > > > > Yeah, it is true "so far". I think there is no problem on current > > > > kernel. > > > > But, unfortunately we need to keep compatibility for old/new DT > > > > (= actually, I don't like this DT rule. It is 100% "shackles for the > > > > legs") > > > > Thus, my big concern is that, in the future, > > > > "if" we added "renesas,ulcb" compatible driver/soc, > > > > both h3/m3 ulcb will use it. > > > > Then, if "h3" can work/boot by using same "m3" settings, it is no > > > > problem > > > > for me (= "works but limited" is also OK, of course. > > > > > > > > This means "matched to more generic compatible") > > > > > > "renesas,ulcb" is very generic naming. > > > Not only h3/m3, if we had v3/e3/d3 etc ulcb, > > > > Furthermore, "ulcb" is an unofficial term, the boards are named "starter > > kit" > > (SK). Using internal names in code or device tree sources is a normal > > practice > > and is fine with me, but I'm a bit bothered by the fact that the H3/M3 > > boards > > are called ULCB in DT, while the V3 board are called SK. I wonder if we > > should > > unify that or if it's too late. > > Perhaps we should. > > Renesas has a long history of boards named SK or RSK. > The inconsistency started when suddenly SK was spelled out in full, with > "Premier" or "Pro" added to differentiate, and the need arose for a shorter > nickname, which became "ULCB" I really appreciate your comments, but it looks like at least the following open questions prevent this series to advance into v2 (feel free to point out flaws in my understanding): - [A] it is not clear if H3ULCB, M3ULCB and M3NULCB boards should use a common compatible string or dedicated ones. - [B] In case a common string is used for all *ULCB boards, should it drop the unofficial "ulcb" (Ultra Low Cost Board, thanks Laurent) in exchange to "sk", "starter-kit" or similar? - [C] Same as [A] and [B], but applied to ULCB DTS filenames, which are currently formed based on the same "ulcb" stem. IMHO these questions go somewhat beyond the scope of M3-N ULCB bring-up. In spite of this, I would be happy to implement your proposals. I am also fine to wait a couple more days to collect more feedback, as well as let the ideas/thoughts to settle. However, if you expect the latter to take longer, maybe we can find some "acceptable" solution and defer the naming issues to a later point? Thanks, Eugeniu.
RE: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize
Hi Geert, Just FYI... On Wednesday, August 08, 2018, Geert Uytterhoeven wrote: > 4. Drop "renesas,scif" from the compatible value in the (not yet > submitted) r7s9210.dtsi (this may have to be clarified in the DT > bindings, although they already say "renesas,scif" is only meant for > ports compatible with the generic version, whatever that means ;-), Just as you would expect, if I drop "renesas,scif" from r7s9210.dtsi, then earlycon works with your RFC patches since now there is only one compatible it can possibly match against. Chris
Re: [PATCH V2 4/5] PCI: rcar: Support runtime PM, link state L1 handling
On 07/25/2018 11:08 PM, Marek Vasut wrote:
> On 06/13/2018 07:25 PM, Bjorn Helgaas wrote:
>> On Wed, Jun 13, 2018 at 04:52:52PM +0100, Lorenzo Pieralisi wrote:
>>> On Wed, Jun 13, 2018 at 08:53:08AM -0500, Bjorn Helgaas wrote:
On Wed, Jun 13, 2018 at 01:54:51AM +0200, Marek Vasut wrote:
> On 06/11/2018 03:59 PM, Bjorn Helgaas wrote:
>> On Sun, Jun 10, 2018 at 03:57:10PM +0200, Marek Vasut wrote:
>>> On 11/17/2017 06:49 PM, Lorenzo Pieralisi wrote:
On Fri, Nov 10, 2017 at 10:58:42PM +0100, Marek Vasut wrote:
> From: Phil Edworthy
>
> Most PCIe host controllers support L0s and L1 power states via ASPM.
> The R-Car hardware only supports L0s, so when the system suspends and
> resumes we have to manually handle L1.
> When the system suspends, cards can put themselves into L1 and send a
I assumed L1 entry has to be negotiated depending upon the PCIe
hierarchy capabilities, I would appreciate if you can explain to
me what's the root cause of the issue please.
>>>
>>> You should probably ignore the suspend/resume part altogether. The issue
>>> here is that the cards can enter L1 state, while the controller won't do
>>> that automatically, it can only detect that the link went into L1 state.
>>> If that happens,the driver must manually put the controller to L1 state.
>>> The controller can transition out of L1 state automatically though.
>>
>> From earlier discussion I thought the R-Car root port did not
>> advertise L1 support.
>
> Which discussion ? This one or somewhere else ?
https://lkml.kernel.org/r/hk2pr0601mb1393d917d343e6363484ca68f5...@hk2pr0601mb1393.apcprd06.prod.outlook.com
Re-reading that, I think I see my misunderstanding. I was only
considering L1 in the ASPM context. I didn't realize the L1
implications of devices being in states other than D0.
Obviously L1 support for ASPM is optional and advertised via Link
Capabilities. But per PCIe r4.0, sec 5.2, L1 support is required for
PCI-PM compatible power management, and is entered "whenever all
Functions ... are programmed to a D-state other than D0."
So I guess this means *every* device is supposed to support L1 when it
is in a non-D0 power state. I think *this* is the case you're
solving.
A little more of this detail, e.g., that this issue has nothing to do
with ASPM, it's probably an R-Car erratum that the RC can't transition
from L1 to L0, etc., in the changelog would really help clear things
up for me.
>>>
>>> I think that the issue is related to the L0->L1 transition upon system
>>> suspend (ie the kernel must force the controller into L1 when all
>>> devices are in a sleep state) and for this specific reason I still think
>>> that checking for a PM_Enter_L1 DLLP reception and doing the L0->L1
>>> transition within a config access is wrong and prone to error (what's
>>> the rationale behind that ?), this ought to be done using PM methods in
>>> the host controller driver.
>>
>> But doesn't the problem happen whenever the link goes to L1, for any
>> reason? E.g., runtime power management might put an endpoint in D3
>> even if we're not doing a whole system suspend. A user could even
>> force the endpoint to D3 by writing to PCI_PM_CTRL with "setpci". If
>> that's the case, I don't think the host controller PM methods will be
>> enough to work around the issue.
>
> I think so, it's the link that goes into L1 state and this can happen
> without any action from the controller side.
>
>> The comment in the patch ("If we are not in L1 link state and we have
>> received PM_ENTER_L1 DLLP, transition to L1 link state") suggests that
>> the R-Car host doesn't handle step 10 in PCIe r4.0, sec 5.3.2.1
>> correctly, i.e., it doesn't complete the transition of the link to L1.
>>
>> Putting this workaround in the config accessor makes sense to me
>> because in this situation the endpoint thinks it's in L1 and it won't
>> receive TLPs for config accesses. Apparently forcing the RP to L1
>> completes the L1 entry, and the RP correctly handles the "Exit from L1
>> State" (sec 5.3.2.2) that's required when the RP needs to send a TLP
>> to the endpoint.
>>
>> I think there's still a potential issue if the endpoint goes to a
>> non-D0 state, the link is stuck in this transitional state (endpoint
>> thinks it's L1, RP thinks it's L0), and the *endpoint* wants to exit
>> L1, e.g., so it can send a PME message for a wakeup. I don't know
>> what happens then.
>
> Is there some hardware which I can use to simulate this situation ?
>
>> If there were a real erratum writeup for this, it would probably
>> discuss this situation.
>
> I went through the latest errata sheet and don't see anything. The
> datasheet only mentions that L0/L0s/L1 is supported and L2 is not supported.
>
> Maybe Phil can comment on
RE: Fix RCAR-V3H SoC wrong IPSR9/10 registers
Hi
Thanks for your review. I'm using link from elinux may be I need to send
It to someone who's maintaining this branch. I have issues sending
patch from cooperate domain via send-email.
This file is called drivers/pinctrl/sh-pfc/pfc-r8a77980.c in upstream Linux.
>>> Thanks
> },
> { PINMUX_CFG_REG("IPSR10", 0xe6060228, 32, 4) {
> - IP8_31_28
> - IP8_27_24
> - IP8_23_20
> - IP8_19_16
> - IP8_15_12
> - IP8_11_8
> - IP8_7_4
> - IP8_3_0 }
> + IP9_31_28
> + IP9_27_24
> + IP9_23_20
> + IP9_19_16
> + IP9_15_12
> + IP9_11_8
> + IP9_7_4
> + IP9_3_0 }
Should't that be IP10_...?
>>> I accidently sent you wrong version of patch. Its fixed in original version
I have.
Regards
Adnan
Renesas Electronics Europe GmbH,Geschaeftsfuehrer/President : Michael
Hannawald, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse
10, 40472 Duesseldorf, Germany,Handelsregister/Commercial Register:
Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406
WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
Re: Fix RCAR-V3H SoC wrong IPSR9/10 registers
Hi Adnan,
On Wed, Aug 8, 2018 at 1:39 PM Adnan Ali wrote:
> Please review this patch.
Thanks for your patch!
But please don't send patches as attachments. It makes review more difficult.
> From 468b28ae35eb0b64db13e3d7d013abaf3734ae32 Mon Sep 17 00:00:00 2001
> From: Adnan Ali
> Date: Wed, 8 Aug 2018 13:14:47 +0200
> Subject: [PATCH] sh-pfc/pfc-r8a7798.c Fix RCAR-V3H condor wrong IPSR9/10 reg
> settings
>
> Signed-off-by: Adnan Ali
> ---
> drivers/pinctrl/sh-pfc/pfc-r8a7798.c | 32
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7798.c
> b/drivers/pinctrl/sh-pfc/pfc-r8a7798.c
> index f6f159e..48584e0 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7798.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7798.c
This file is called drivers/pinctrl/sh-pfc/pfc-r8a77980.c in upstream Linux.
> @@ -3122,24 +3122,24 @@ static const struct pinmux_cfg_reg
> pinmux_config_regs[] = {
> IP8_3_0 }
> },
> { PINMUX_CFG_REG("IPSR9", 0xe6060224, 32, 4) {
> - IP8_31_28
> - IP8_27_24
> - IP8_23_20
> - IP8_19_16
> - IP8_15_12
> - IP8_11_8
> - IP8_7_4
> - IP8_3_0 }
> + IP9_31_28
> + IP9_27_24
> + IP9_23_20
> + IP9_19_16
> + IP9_15_12
> + IP9_11_8
> + IP9_7_4
> + IP9_3_0 }
This bug is not present in the upstream version:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/sh-pfc/pfc-r8a77980.c#n2728
> },
> { PINMUX_CFG_REG("IPSR10", 0xe6060228, 32, 4) {
> - IP8_31_28
> - IP8_27_24
> - IP8_23_20
> - IP8_19_16
> - IP8_15_12
> - IP8_11_8
> - IP8_7_4
> - IP8_3_0 }
> + IP9_31_28
> + IP9_27_24
> + IP9_23_20
> + IP9_19_16
> + IP9_15_12
> + IP9_11_8
> + IP9_7_4
> + IP9_3_0 }
Should't that be IP10_...?
Also not present in the upstream version.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RE: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize
Hi Geert,
On Wednesday, August 08, 2018, Geert Uytterhoeven wrote:
> Suggestions for actions:
> 1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
> SCIx_RZ_SCIFA_REGTYPE"), as this is a dependency for step 2,
> 2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
> SCIF address"), to unbreak earlycon on RZ/A1, RZ/G, and R-Car SCIF
> ports,
Oh well. The idea of a shared SCIF regtype was a good idea...just too
much baggage to deal with.
> 4. Drop "renesas,scif" from the compatible value in the (not yet
> submitted) r7s9210.dtsi (this may have to be clarified in the DT
> bindings, although they already say "renesas,scif" is only meant for
> ports compatible with the generic version, whatever that means ;-),
As soon as I update to the new clock driver style and get that
mainlined, then I can submit the r7s9210.dtsi.
> 5. Add an OF_EARLYCON_DECLARE() for RZ/A2, setting port_cfg.regtype to
> SCIx_RZ_SCIFA_REGTYPE, to (presumably) make earlycon work on RZ/A2,
This is the one that I was happy to learn how to do it since originally
I wasn't sure how I could get earlycon to work with the oddball RZ/A2
SCIF.
Thanks,
Chris
Re: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value
Hi Chris,
On Wed, Aug 8, 2018 at 12:39 PM Chris Brandt wrote:
> On Wednesday, August 08, 2018, Geert Uytterhoeven wrote:
> > BTW, I guess earlycon also works on RZ/A2 with current tty-next or
> > latest renesas-drivers?
>
> I was using your RFC patches on top of the current renesas-drivers.
I meant without the RFC patch series.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH/RFC 0/4] sh-sci : Do not derive regshift from regsize
On Mon, Aug 6, 2018 at 4:08 PM Geert Uytterhoeven
wrote:
> This RFC patch series was sparked by noticing that commit 2d4dd0da45401c7a
> ("serial: sh-sci: Allow for compressed SCIF address") broke earlycon
> support on most Renesas ARM SoCs using SCIF ports, and by the fragility of
> deriving regshift from the register block size (which may be rounded up):
> 1. The first patch is an old patch from Sato-san, which I never really
> understood. But it turned out to be a dependency for patch 2.
> 2. Patch 2 makes sure regshift is initialized when using earlycon,
> unbreaking the serial console on e.g. R-Car Gen2 and Gen3.
> 3. Patch 3 reverts the patch that started deriving regshift from the
> register block size, and that removed the plat_sci_port.regshift
> field. Which is a field I needed again in patch 4.
> 4. Patch 4 removes the remaining regshift derivations on DT platforms.
> (5. I didn't bother writing patch 5, which involves adding .regshift
> initializations to all SH board files that need it.)
>
> However, I'm not happy with the end result, so please DO NOT apply this!
> As I spent almost a full day on this, and would still like to know the
> story about "sh-sci: Use a separate sci_port for earlycon", I decided to
> post it anyway.
>
> As earlycon will be broken in v4.19-rc1 on RZ/A1, RZ/G, and R-Car, assuming
> no other actions are taken, an alternative solution would be to:
> 1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
> SCIx_RZ_SCIFA_REGTYPE"),
> 2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
> SCIF address") alternative,
> 3. Add an OF_EARLYCON_DECLARE() for RZ/A2, to fix earlycon on RZ/A2.
>
> What do you think?
> Thanks for your comments!
I gave this a bit more thought, and these are my conclusions:
1. RZ/A2 SCIF is not really compatible with SCIF on RZ/A1, RZ/G, and
R-Car, due to the absence of support for the DT "reg-shift" property.
Given RZ/A1 and R-Car would need "reg-shift = <1>", we cannot
retroactively add support for that for ports declared compatible with
"renesas,scif" (assuming regshift == 1 in case the "reg-shift"
property is not present may work for the main serial driver, but not
for earlycon, where of_setup_earlycon() is in charge of all register
block parsing).
Sorry for suggesting this in the first place. Based on the regshift
handling for SCI, I assumed it was fine, but apparently that
particular code isn't without problems on SCI ports neither.
2. SCI ports are the only ports where a DT "reg-shift" property makes
sense, as it is the only port type with fixed (8-bit) register sizes,
with registers spaced 1, 2 or 4 bytes apart (regshift = 0, 1, or 2).
All other types use a mix of 8-bit and 16-bit registers, 2 or 4 bytes
apart.
- On H8/300 (DT only), regshift is always zero.
AFAIK this is handled correctly for earlycon, but not for the main
serial driver (presumably broken since commit
dfc80387aefb78161f83732804c6d01c89c24595, cfr. Sato-san's patch
"serial: sh-sci: byte allocated register support"
(https://www.spinics.net/lists/linux-sh/msg53175.html)).
- On SuperH (non-DT), regshift is 1 or 2.
If/when DT support is ever added, the "reg-shift" DT property can
be used to indicate that.
Suggestions for actions:
1. Revert commit 7acece71a517cad8 ("serial: sh-sci: Remove
SCIx_RZ_SCIFA_REGTYPE"), as this is a dependency for step 2,
2. Revert commit 2d4dd0da45401c7a ("serial: sh-sci: Allow for compressed
SCIF address"), to unbreak earlycon on RZ/A1, RZ/G, and R-Car SCIF
ports,
3. Restrict the regshift setup (1 or 2) for PORT_SCI to the
!dev->dev.of_node case, to fix SCI ports instantiated from DT on
H8/300.
4. Drop "renesas,scif" from the compatible value in the (not yet
submitted) r7s9210.dtsi (this may have to be clarified in the DT
bindings, although they already say "renesas,scif" is only meant for
ports compatible with the generic version, whatever that means ;-),
5. Add an OF_EARLYCON_DECLARE() for RZ/A2, setting port_cfg.regtype to
SCIx_RZ_SCIFA_REGTYPE, to (presumably) make earlycon work on RZ/A2,
1-2 are regression fixes that can be done immediately,
3 is a bug fix to be submitted,
5 is an enhancement to be submitted.
Any other opinions or comments?
As both Greg and I will be on holidays until after the v4.18 release, you
have a bit more time to polish all of this for the serial pull request
for v4.19...
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
RE: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value
Hi Geert, On Wednesday, August 08, 2018, Geert Uytterhoeven wrote: > BTW, I guess earlycon also works on RZ/A2 with current tty-next or > latest renesas-drivers? I was using your RFC patches on top of the current renesas-drivers. Chris
Re: [PATCH/RFC 4/4] sh-sci: Derive regshift value from DT compatible value
Hi Chris,
On Wed, Aug 8, 2018 at 2:16 AM Chris Brandt wrote:
> On Tuesday, August 07, 2018, Geert Uytterhoeven wrote:
> > > I see that you added this:
> > >
> > > OF_EARLYCON_DECLARE(scif, "renesas,scif-r7s9210",
> > rza2_early_console_setup);
> > >
> > > and "renesas,scif-r7s9210" matches what I have in my .dtsi.
> > >
> > > But, when I run the code, I end up in function scif_early_console_setup,
> > > not rza2_early_console_setup
> >
> > Hmm, perhaps it doesn't pick the first/most specialized match.
>
> It seems it picks the first match.
>
> But, the table is built in the opposite order in which they are declared
> in the file. So "renesas,scif" was coming before
> "renesas,scif-r7s9210".
>
> So, by just swapping the order of "renesas,scif" and
> "renesas,scif-r7s9210" made it work!
Right, and since the RZ/A2 SCIF is not really compatible with
"renesas,scif" (see my conclusion as a reply to the cover letter), the
other order doesn't work.
BTW, I guess earlycon also works on RZ/A2 with current tty-next or
latest renesas-drivers?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH 1/2] i2c: rcar: refactor private flags
Use BIT macro to avoid shift-31-problem, indent a little more and use
GENMASK to make it easier to add new flags.
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-rcar.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 791a4aa34fdd..a9f1880e2eae 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -19,6 +19,7 @@
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*/
+#include
#include
#include
#include
@@ -112,9 +113,9 @@
#define ID_ARBLOST (1 << 3)
#define ID_NACK(1 << 4)
/* persistent flags */
-#define ID_P_NO_RXDMA (1 << 30) /* HW forbids RXDMA sometimes */
-#define ID_P_PM_BLOCKED(1 << 31)
-#define ID_P_MASK (ID_P_PM_BLOCKED | ID_P_NO_RXDMA)
+#define ID_P_NO_RXDMA BIT(30) /* HW forbids RXDMA sometimes */
+#define ID_P_PM_BLOCKEDBIT(31)
+#define ID_P_MASK GENMASK(31, 30)
enum rcar_i2c_type {
I2C_RCAR_GEN1,
--
2.11.0
[PATCH 2/2] i2c: rcar: implement STOP and REP_START according to docs
When doing a REP_START after a read message, the driver used to trigger
a STOP first which would then be overwritten by REP_START. This was the
only stable method found when doing the last refactoring. However, this
was not in accordance with the documentation.
After research from our BSP team and myself, we now can implement a
version which works and is according to the documentation. The new
approach ensures the ICMCR register is only changed when really needed.
Tested on a R-Car Gen2 (H2) and Gen3 with DMA (M3N).
Signed-off-by: Hiromitsu Yamasaki
Signed-off-by: Wolfram Sang
---
drivers/i2c/busses/i2c-rcar.c | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index a9f1880e2eae..43ad933df0f0 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -113,9 +113,10 @@
#define ID_ARBLOST (1 << 3)
#define ID_NACK(1 << 4)
/* persistent flags */
+#define ID_P_REP_AFTER_RD BIT(29)
#define ID_P_NO_RXDMA BIT(30) /* HW forbids RXDMA sometimes */
#define ID_P_PM_BLOCKEDBIT(31)
-#define ID_P_MASK GENMASK(31, 30)
+#define ID_P_MASK GENMASK(31, 29)
enum rcar_i2c_type {
I2C_RCAR_GEN1,
@@ -345,7 +346,10 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv
*priv)
rcar_i2c_write(priv, ICMSR, 0);
rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
} else {
- rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
+ if (priv->flags & ID_P_REP_AFTER_RD)
+ priv->flags &= ~ID_P_REP_AFTER_RD;
+ else
+ 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);
@@ -550,15 +554,15 @@ static void rcar_i2c_irq_recv(struct rcar_i2c_priv *priv,
u32 msr)
priv->pos++;
}
- /*
-* If next received data is the _LAST_, go to STOP phase. Might be
-* overwritten by REP START when setting up a new msg. Not elegant
-* but the only stable sequence for REP START I have found so far.
-* If you want to change this code, make sure sending one transfer with
-* four messages (WR-RD-WR-RD) works!
-*/
- if (priv->pos + 1 >= msg->len)
- rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
+ /* If next received data is the _LAST_, go to new phase. */
+ if (priv->pos + 1 == msg->len) {
+ if (priv->flags & ID_LAST_MSG) {
+ rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
+ } else {
+ rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
+ priv->flags |= ID_P_REP_AFTER_RD;
+ }
+ }
if (priv->pos == msg->len && !(priv->flags & ID_LAST_MSG))
rcar_i2c_next_msg(priv);
@@ -626,9 +630,11 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
struct rcar_i2c_priv *priv = ptr;
u32 msr, val;
- /* Clear START or STOP as soon as we can */
- val = rcar_i2c_read(priv, ICMCR);
- rcar_i2c_write(priv, ICMCR, val & RCAR_BUS_MASK_DATA);
+ /* Clear START or STOP immediately, except for REPSTART after read */
+ if (likely(!(priv->flags & ID_P_REP_AFTER_RD))) {
+ val = rcar_i2c_read(priv, ICMCR);
+ rcar_i2c_write(priv, ICMCR, val & RCAR_BUS_MASK_DATA);
+ }
msr = rcar_i2c_read(priv, ICMSR);
--
2.11.0
[PATCH 0/2] i2c: rcar: implement STOP and REP_START according to docs
Details are in the patch descriptions. Thanks to Yamasaki-san, Shimoda-san, and all other people involved. Wolfram Sang (2): i2c: rcar: refactor private flags i2c: rcar: implement STOP and REP_START according to docs drivers/i2c/busses/i2c-rcar.c | 39 +++ 1 file changed, 23 insertions(+), 16 deletions(-) -- 2.11.0
