Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Robin Gong <yibin.gong@nxp.com>
To:     Martin Fuzzey <martin.fuzzey@flowbird.group>,
        "dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>
CC:     "stable@vger.kernel.org" <stable@vger.kernel.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        dl-linux-imx <linux-imx@nxp.com>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] dmaengine: imx-sdma: fix context cache
Thread-Topic: [PATCH] dmaengine: imx-sdma: fix context cache
Thread-Index: AQHV1qnIaNGAJ9R9HEaS0rzYiAQ7rag1GlnQ
Date:   Mon, 2 Mar 2020 07:36:54 +0000
Message-ID: <VE1PR04MB66383055663F08FBFDF2E47489E70@VE1PR04MB6638.eurprd04.prod.outlook.com>
References: <1580305274-27274-1-git-send-email-martin.fuzzey@flowbird.group>
In-Reply-To: <1580305274-27274-1-git-send-email-martin.fuzzey@flowbird.group>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 574c4a10-eb01-459e-cb2d-08d7be7c7aeb
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Mar 2020 07:36:54.1990
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: aRHXWh+CsUO01BTgF1us9wvY+sqTuQgECo8Lj6yoe80qVb8vekEf/VV80Hkf/z2pEQIL4aoxa6rPiGxXoZy1ew==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB6445
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/01/29 Martin Fuzzey <martin.fuzzey@flowbird.group> wrote:
>=20
> There is a DMA problem with the serial ports on i.MX6.
>=20
> When the following sequence is performed:
>=20
> 1) Open a port
> 2) Write some data
> 3) Close the port
> 4) Open a *different* port
> 5) Write some data
> 6) Close the port
>=20
> The second write sends nothing and the second close hangs.
> If the first close() is omitted it works.
>=20
> Adding logs to the the UART driver shows that the DMA is being setup but =
the
> callback is never invoked for the second write.
>=20
> This used to work in 4.19.
>=20
> Git bisect leads to:
> 	ad0d92d: "dmaengine: imx-sdma: refine to load context only once"
>=20
> This commit adds a "context_loaded" flag used to avoid unnecessary contex=
t
> setups.
> However the flag is only reset in sdma_channel_terminate_work(), which is
> only invoked in a worker triggered by sdma_terminate_all() IF there is an=
 active
> descriptor.
>=20
> So, if no active descriptor remains when the channel is terminated, the f=
lag is
> not reset and, when the channel is later reused the old context is used.
>=20
> Fix the problem by always resetting the flag in sdma_free_chan_resources(=
).
>=20
> Fixes: ad0d92d: "dmaengine: imx-sdma: refine to load context only once"
> Cc: stable@vger.kernel.org
> Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
>=20
> ---
>=20
> The following python script may be used to reproduce the problem:
>=20
> import re, serial, sys
>=20
> ports=3D(0, 4) # Can be any ports not used (no need to connect anything) =
NOT
> console...
>=20
> def get_tx_counts():
>         pattern =3D re.compile("(\d+):.*tx:(\d+).*")
>         tx_counts =3D {}
>         with open("/proc/tty/driver/IMX-uart", "r") as f:
>                 for line in f:
>                         match =3D pattern.match(line)
>                         if match:
>                                 tx_counts[int(match.group(1))] =3D
> int(match.group(2))
>         return tx_counts
>=20
> before =3D get_tx_counts()
>=20
> a =3D serial.Serial("/dev/ttymxc%d" % ports[0])
> a.write("polop")
> a.close()
> b =3D serial.Serial("/dev/ttymxc%d" % ports[1])
> b.write("test")
>=20
> after =3D get_tx_counts()
>=20
> if (after[ports[0]] - before[ports[0]]  > 0) and (after[ports[1]] - befor=
e[ports[1]] >
> 0):
>         print "PASS"
>         sys.exit(0)
> else:
>         print "FAIL"
>         print "Before: %s" % before
>         print "After: %s" % after
>         sys.exit(1)
> ---
>  drivers/dma/imx-sdma.c | 1 +
>  1 file changed, 1 insertion(+)
>=20
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index
> 066b21a..332ca50 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1338,6 +1338,7 @@ static void sdma_free_chan_resources(struct
> dma_chan *chan)
>=20
>  	sdmac->event_id0 =3D 0;
>  	sdmac->event_id1 =3D 0;
> +	sdmac->context_loaded =3D false;
Martin, thanks for you patch, sorry for the issue left in kernel for so lon=
g, because my below patch set has been pending from last year. I would like=
 revert commit ad0d92d: "dmaengine: imx-sdma: refine to load context only o=
nce" since some drivers may change.
context during two transfer like spi did. I would pick up this patch set th=
is week anyway.=20
https://lore.kernel.org/patchwork/patch/1086454/=20
>=20
>  	sdma_set_channel_priority(sdmac, 0);
>=20
> --
> 1.9.1