Re: [PATCH 0/4] remoteproc: k3-r5: Introduce suspend to ram support
Le 01/07/2024 à 11:59, Hari Nagalla a écrit : On 6/21/24 10:00, Richard Genoud wrote: Richard Genoud (4): remoteproc: k3-r5: Fix IPC-only mode detection remoteproc: k3-r5: Introduce PM suspend/resume handlers remoteproc: k3-r5: k3_r5_rproc_stop: code reorder remoteproc: k3-r5: support for graceful stop of remote cores IMO, the patches are better reordered as below (since there is a LPM rearch in progress) patch1 - Independent of other patches and is clearly a bug fix. patch3,4 - Support for graceful shutdown. A separate feature used by customers wanting to change FW at runtime and is independent of suspend/resume. patch 2 - suspend/resume handlers.. Indeed, patches 2, 3, 4 still need some internal discussions. But I think patch 1 could be taken as is, since it's a bug fix. Regards, Richard
Re: [PATCH 3/4] remoteproc: k3-r5: k3_r5_rproc_stop: code reorder
Le 01/07/2024 à 18:35, Mathieu Poirier a écrit :
On Mon, Jul 01, 2024 at 10:03:22AM +0200, Richard GENOUD wrote:
Le 28/06/2024 à 23:18, Mathieu Poirier a écrit :
On Fri, Jun 21, 2024 at 05:00:57PM +0200, Richard Genoud wrote:
In the next commit, a RP_MBOX_SHUTDOWN message will be sent in
k3_r5_rproc_stop() to the remote proc (in lockstep on not)
Thus, the sanity check "do not allow core 0 to stop before core 1"
should be moved at the beginning of the function so that the generic case
can be dealt with.
In order to have an easier patch to review, those actions are broke in
two patches:
- this patch: moving the sanity check at the beginning (No functional
change).
- next patch: doing the real job (sending shutdown messages to remote
procs before halting them).
Basically, we had:
- cluster_mode actions
- !cluster_mode sanity check
- !cluster_mode actions
And now:
- !cluster_mode sanity check
- cluster_mode actions
- !cluster_mode actions
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,16 +636,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
- /* halt all applicable cores */
- if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
- list_for_each_entry(core, &cluster->cores, elem) {
- ret = k3_r5_core_halt(core);
- if (ret) {
- core = list_prev_entry(core, elem);
- goto unroll_core_halt;
- }
- }
- } else {
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
@@ -656,6 +648,18 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
ret = -EPERM;
goto out;
}
+ }
+
+ /* halt all applicable cores */
+ if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
+ list_for_each_entry(core, &cluster->cores, elem) {
+ ret = k3_r5_core_halt(core);
+ if (ret) {
+ core = list_prev_entry(core, elem);
+ goto unroll_core_halt;
+ }
+ }
+ } else {
ret = k3_r5_core_halt(core);
if (ret)
With this patch, the "else" in this "if" condition is coupled with the "if" from
the lockstep mode, making the code extremaly hard to read. The original code
has a k3_r5_core_halt() in both "if" conditions, making the condition
independent from one another.
I'm not sure I understand what you mean.
With your patch applied I get the following: https://pastebin.com/yTZ0pKcS
Let's say the R5 is in split mode and k3_r5_rproc_stop() called on core1. The
if() that deal with that condition is on line 10, while the function that halts
the remote processor is online 34, part of the else clause that handles lockstep
mode. The two if() clauses are entangled and nothing good can come out of that.
Ok, I see your point now.
Thanks !
Anyway, I'm not happy with this diff, it doesn't reflect what was intended.
(which is moving the check "core 0 should not be stop before core 1" at the
beginning)
Tweaking around with the diff algorithms, I came with something way easier to
read I think:
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,6 +636,20 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
+ /* do not allow core 0 to stop before core 1 */
+ core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
+ elem);
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
+ dev_err(dev, "%s: can not stop core 0 before core 1\n",
+ __func__);
+ ret = -EPERM;
+ goto out;
+ }
+
Re: [PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
Le 29/06/2024 à 00:50, Andrew Davis a écrit :
On 6/21/24 10:00 AM, Richard Genoud wrote:
Introduce software IPC handshake between the K3-R5 remote proc driver
and the R5 MCU to gracefully stop/reset the remote core.
Upon a stop request, K3-R5 remote proc driver sends a RP_MBOX_SHUTDOWN
mailbox message to the remote R5 core.
The remote core is expected to:
- relinquish all the resources acquired through Device Manager (DM)
- disable its interrupts
- send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK
- enter WFI state.
Meanwhile, the K3-R5 remote proc driver does:
- wait for the RP_MBOX_SHUTDOWN_ACK from the remote core
- wait for the remote proc to enter WFI state
- reset the remote core through device manager
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/omap_remoteproc.h | 9 +-
drivers/remoteproc/ti_k3_r5_remoteproc.c | 40
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/omap_remoteproc.h
b/drivers/remoteproc/omap_remoteproc.h
index 828e13256c02..c008f11fa2a4 100644
--- a/drivers/remoteproc/omap_remoteproc.h
+++ b/drivers/remoteproc/omap_remoteproc.h
@@ -42,6 +42,11 @@
* @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote
processor
* on a suspend request
*
+ * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor
+ *
+ * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor
for a
+ * shutdown request. The remote processor should be in WFI state
short after.
+ *
* Introduce new message definitions if any here.
*
* @RP_MBOX_END_MSG: Indicates end of known/defined messages from
remote core
@@ -59,7 +64,9 @@ enum omap_rp_mbox_messages {
RP_MBOX_SUSPEND_SYSTEM = 0xFF11,
RP_MBOX_SUSPEND_ACK = 0xFF12,
RP_MBOX_SUSPEND_CANCEL = 0xFF13,
- RP_MBOX_END_MSG = 0xFF14,
+ RP_MBOX_SHUTDOWN = 0xFF14,
+ RP_MBOX_SHUTDOWN_ACK = 0xFF15,
+ RP_MBOX_END_MSG = 0xFF16,
};
#endif /* _OMAP_RPMSG_H */
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index a2ead87952c7..918a15e1dd9a 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -172,8 +173,23 @@ struct k3_r5_rproc {
struct k3_r5_core *core;
struct k3_r5_mem *rmem;
int num_rmems;
+ struct completion shutdown_complete;
};
+/*
+ * This will return true if the remote core is in Wait For Interrupt
state.
+ */
+static bool k3_r5_is_core_in_wfi(struct k3_r5_core *core)
+{
+ int ret;
+ u64 boot_vec;
+ u32 cfg, ctrl, stat;
+
+ ret = ti_sci_proc_get_status(core->tsp, &boot_vec, &cfg, &ctrl,
&stat);
+
+ return !ret ? !!(stat & PROC_BOOT_STATUS_FLAG_R5_WFI) : false;
Too fancy for me :) Just return if (ret) right after get_status().
Ok, too much punctuation :)
Looks like this function is called in a polling loop, if
ti_sci_proc_get_status() fails once, it won't get better,
no need to keep checking, we should just error out of
the polling loop.
Ok
Thanks !
Andrew
+}
+
/**
* k3_r5_rproc_mbox_callback() - inbound mailbox message handler
* @client: mailbox client pointer used for requesting the mailbox
channel
@@ -209,6 +225,10 @@ static void k3_r5_rproc_mbox_callback(struct
mbox_client *client, void *data)
case RP_MBOX_ECHO_REPLY:
dev_info(dev, "received echo reply from %s\n", name);
break;
+ case RP_MBOX_SHUTDOWN_ACK:
+ dev_dbg(dev, "received shutdown_ack from %s\n", name);
+ complete(&kproc->shutdown_complete);
+ break;
default:
/* silently handle all other valid messages */
if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
@@ -634,6 +654,7 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_cluster *cluster = kproc->cluster;
struct device *dev = kproc->dev;
struct k3_r5_core *core1, *core = kproc->core;
+ bool wfi;
int ret;
@@ -650,6 +671,24 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
+ /* Send SHUTDOWN message to remote proc */
+ reinit_completion(&kproc->shutdown_complete);
+ ret = mbox_send_message(kproc->mbox, (void *)RP_MBOX_SHUTDOWN);
+ if (ret < 0) {
+ dev_err(dev, "Sending SHUTDOWN message failed: %d. Halting
core anyway.\n", ret);
+ } else {
+ ret = wait_for_completion_timeout(&kproc->shutdown_complete,
+ msecs_to_jiffies(1000));
+ if (ret == 0) {
+ dev_err(dev, "Timeout waiting SHUTDOWN_ACK message.
Halting core anyway.\n");
+ } else {
+ ret = readx_poll_timeout(k3_r5_is_core_in_wfi, core,
+
Re: [PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
[adding Vibhore in Cc]
Le 28/06/2024 à 23:20, Mathieu Poirier a écrit :
On Fri, Jun 21, 2024 at 05:00:58PM +0200, Richard Genoud wrote:
Introduce software IPC handshake between the K3-R5 remote proc driver
and the R5 MCU to gracefully stop/reset the remote core.
Upon a stop request, K3-R5 remote proc driver sends a RP_MBOX_SHUTDOWN
mailbox message to the remote R5 core.
The remote core is expected to:
- relinquish all the resources acquired through Device Manager (DM)
- disable its interrupts
- send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK
- enter WFI state.
Meanwhile, the K3-R5 remote proc driver does:
- wait for the RP_MBOX_SHUTDOWN_ACK from the remote core
- wait for the remote proc to enter WFI state
- reset the remote core through device manager
Based on work from: Hari Nagalla
Why is this needed now and what happens to system with a new kernel driver and
an older K3R5 firmware?
Without this patch, the IPC firwmares from recent TI PDK prevent the
suspend:
platform 5d0.r5f: ti-sci processor set_control failed: -19
remoteproc remoteproc1: can't stop rproc: -19
platform 5c0.r5f: Failed to shutdown rproc (-19)
platform 5c0.r5f: k3_r5_rproc_suspend failed (-19)
With a new kernel driver and an old firmware, this will add a timeout
before stopping it, and the message:
platform 5c0.r5f: Timeout waiting SHUTDOWN_ACK message. Halting core
anyway.
(tested on old FW 09.00.00.01 and new FW 09.02.01.18, on J7200)
Regards,
Richard
Thanks,
Mathieu
Signed-off-by: Richard Genoud
---
drivers/remoteproc/omap_remoteproc.h | 9 +-
drivers/remoteproc/ti_k3_r5_remoteproc.c | 40
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/omap_remoteproc.h
b/drivers/remoteproc/omap_remoteproc.h
index 828e13256c02..c008f11fa2a4 100644
--- a/drivers/remoteproc/omap_remoteproc.h
+++ b/drivers/remoteproc/omap_remoteproc.h
@@ -42,6 +42,11 @@
* @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor
* on a suspend request
*
+ * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor
+ *
+ * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a
+ * shutdown request. The remote processor should be in WFI state short after.
+ *
* Introduce new message definitions if any here.
*
* @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core
@@ -59,7 +64,9 @@ enum omap_rp_mbox_messages {
RP_MBOX_SUSPEND_SYSTEM = 0xFF11,
RP_MBOX_SUSPEND_ACK = 0xFF12,
RP_MBOX_SUSPEND_CANCEL = 0xFF13,
- RP_MBOX_END_MSG = 0xFF14,
+ RP_MBOX_SHUTDOWN= 0xFF14,
+ RP_MBOX_SHUTDOWN_ACK= 0xFF15,
+ RP_MBOX_END_MSG = 0xFF16,
};
#endif /* _OMAP_RPMSG_H */
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index a2ead87952c7..918a15e1dd9a 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -172,8 +173,23 @@ struct k3_r5_rproc {
struct k3_r5_core *core;
struct k3_r5_mem *rmem;
int num_rmems;
+ struct completion shutdown_complete;
};
+/*
+ * This will return true if the remote core is in Wait For Interrupt state.
+ */
+static bool k3_r5_is_core_in_wfi(struct k3_r5_core *core)
+{
+ int ret;
+ u64 boot_vec;
+ u32 cfg, ctrl, stat;
+
+ ret = ti_sci_proc_get_status(core->tsp, &boot_vec, &cfg, &ctrl, &stat);
+
+ return !ret ? !!(stat & PROC_BOOT_STATUS_FLAG_R5_WFI) : false;
+}
+
/**
* k3_r5_rproc_mbox_callback() - inbound mailbox message handler
* @client: mailbox client pointer used for requesting the mailbox channel
@@ -209,6 +225,10 @@ static void k3_r5_rproc_mbox_callback(struct mbox_client
*client, void *data)
case RP_MBOX_ECHO_REPLY:
dev_info(dev, "received echo reply from %s\n", name);
break;
+ case RP_MBOX_SHUTDOWN_ACK:
+ dev_dbg(dev, "received shutdown_ack from %s\n", name);
+ complete(&kproc->shutdown_complete);
+ break;
default:
/* silently handle all other valid messages */
if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
@@ -634,6 +654,7 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_cluster *cluster = kproc->cluster;
struct device *dev = kproc->dev;
struct k3_r5_core *core1, *core = kproc->core;
+ bool wfi;
int ret;
@@ -650,6 +671,24 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
+ /* Send SHUTDOWN message to remote proc */
+ reinit_completion(&kproc->shutdown_complete);
+
Re: [PATCH 3/4] remoteproc: k3-r5: k3_r5_rproc_stop: code reorder
Le 28/06/2024 à 23:18, Mathieu Poirier a écrit :
On Fri, Jun 21, 2024 at 05:00:57PM +0200, Richard Genoud wrote:
In the next commit, a RP_MBOX_SHUTDOWN message will be sent in
k3_r5_rproc_stop() to the remote proc (in lockstep on not)
Thus, the sanity check "do not allow core 0 to stop before core 1"
should be moved at the beginning of the function so that the generic case
can be dealt with.
In order to have an easier patch to review, those actions are broke in
two patches:
- this patch: moving the sanity check at the beginning (No functional
change).
- next patch: doing the real job (sending shutdown messages to remote
procs before halting them).
Basically, we had:
- cluster_mode actions
- !cluster_mode sanity check
- !cluster_mode actions
And now:
- !cluster_mode sanity check
- cluster_mode actions
- !cluster_mode actions
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,16 +636,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
- /* halt all applicable cores */
- if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
- list_for_each_entry(core, &cluster->cores, elem) {
- ret = k3_r5_core_halt(core);
- if (ret) {
- core = list_prev_entry(core, elem);
- goto unroll_core_halt;
- }
- }
- } else {
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
@@ -656,6 +648,18 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
ret = -EPERM;
goto out;
}
+ }
+
+ /* halt all applicable cores */
+ if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
+ list_for_each_entry(core, &cluster->cores, elem) {
+ ret = k3_r5_core_halt(core);
+ if (ret) {
+ core = list_prev_entry(core, elem);
+ goto unroll_core_halt;
+ }
+ }
+ } else {
ret = k3_r5_core_halt(core);
if (ret)
With this patch, the "else" in this "if" condition is coupled with the "if" from
the lockstep mode, making the code extremaly hard to read. The original code
has a k3_r5_core_halt() in both "if" conditions, making the condition
independent from one another.
I'm not sure I understand what you mean.
Anyway, I'm not happy with this diff, it doesn't reflect what was intended.
(which is moving the check "core 0 should not be stop before core 1" at the
beginning)
Tweaking around with the diff algorithms, I came with something way easier to
read I think:
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,6 +636,20 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
+ /* do not allow core 0 to stop before core 1 */
+ core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
+ elem);
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
+ dev_err(dev, "%s: can not stop core 0 before core 1\n",
+ __func__);
+ ret = -EPERM;
+ goto out;
+ }
+ }
+
/* halt all applicable cores */
if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
list_for_each_entry(core, &cluster->cores, elem) {
@@ -646,16 +660,6 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
} else {
- /* do not allow core 0 to stop before core 1 */
- core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
- elem);
- if (core != core1 &&
Re: [PATCH 2/4] remoteproc: k3-r5: Introduce PM suspend/resume handlers
Le 28/06/2024 à 22:48, Mathieu Poirier a écrit :
On Fri, Jun 21, 2024 at 05:00:56PM +0200, Richard Genoud wrote:
This patch adds the support for system suspend/resume to the ti_k3_R5
remoteproc driver.
In order to save maximum power, the approach here is to shutdown
completely the cores that were started by the kernel (i.e. those in
RUNNING state).
Those which were started before the kernel (in attached mode) will be
detached.
The pm_notifier mechanism is used here because the remote procs firmwares
have to be reloaded at resume, and thus the driver must have access to
the file system were the firmware is stored.
On suspend, the running remote procs are stopped, the attached remote
procs are detached and processor control released.
On resume, the reverse operation is done.
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 123 ++-
1 file changed, 121 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 39a47540c590..1f18b08618c8 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -112,6 +113,7 @@ struct k3_r5_cluster {
struct list_head cores;
wait_queue_head_t core_transition;
const struct k3_r5_soc_data *soc_data;
+ struct notifier_block pm_notifier;
};
/**
@@ -577,7 +579,8 @@ static int k3_r5_rproc_start(struct rproc *rproc)
/* do not allow core 1 to start before core 0 */
core0 = list_first_entry(&cluster->cores, struct k3_r5_core,
elem);
- if (core != core0 && core0->rproc->state == RPROC_OFFLINE) {
+ if (core != core0 && (core0->rproc->state == RPROC_OFFLINE ||
+ core0->rproc->state == RPROC_SUSPENDED)) {
If I understand correctly, this is to address a possible race condition between
user space wanting to start core1 via sysfs while the system is being suspended.
Is this correct? If so, please add a comment to explain what is going on.
Otherwise a comment is obviously needed.
Yes, you're right, I'll add a comment on the race condition at suspend.
dev_err(dev, "%s: can not start core 1 before core 0\n",
__func__);
ret = -EPERM;
@@ -646,7 +649,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
- if (core != core1 && core1->rproc->state != RPROC_OFFLINE) {
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
dev_err(dev, "%s: can not stop core 0 before core 1\n",
__func__);
ret = -EPERM;
@@ -1238,6 +1242,117 @@ static int k3_r5_rproc_configure_mode(struct
k3_r5_rproc *kproc)
return ret;
}
+static int k3_r5_rproc_suspend(struct k3_r5_rproc *kproc)
+{
+ unsigned int rproc_state = kproc->rproc->state;
+ int ret;
+
+ if (rproc_state != RPROC_RUNNING && rproc_state != RPROC_ATTACHED)
+ return 0;
+
+ if (rproc_state == RPROC_RUNNING)
+ ret = rproc_shutdown(kproc->rproc);
+ else
+ ret = rproc_detach(kproc->rproc);
+
+ if (ret) {
+ dev_err(kproc->dev, "Failed to %s rproc (%d)\n",
+ (rproc_state == RPROC_RUNNING) ? "shutdown" : "detach",
+ ret);
+ return ret;
+ }
+
+ kproc->rproc->state = RPROC_SUSPENDED;
+
+ return ret;
+}
+
+static int k3_r5_rproc_resume(struct k3_r5_rproc *kproc)
+{
+ int ret;
+
+ if (kproc->rproc->state != RPROC_SUSPENDED)
+ return 0;
+
+ ret = k3_r5_rproc_configure_mode(kproc);
+ if (ret < 0)
+ return -EBUSY;
+
+ /*
+* ret > 0 for IPC-only mode
+* ret == 0 for remote proc mode
+*/
+ if (ret == 0) {
+ /*
+* remote proc looses its configuration when powered off.
+* So, we have to configure it again on resume.
+*/
+ ret = k3_r5_rproc_configure(kproc);
+ if (ret < 0) {
+ dev_err(kproc->dev, "k3_r5_rproc_configure failed
(%d)\n", ret);
+ return -EBUSY;
+
[PATCH 1/4] remoteproc: k3-r5: Fix IPC-only mode detection
ret variable was used to test reset status, get from
reset_control_status() call. But this variable was overwritten by
ti_sci_proc_get_status() a few lines bellow.
And as ti_sci_proc_get_status() returns 0 or a negative value (in this
latter case, followed by a return), the expression !ret was always true,
Clearly, this was not what was intended:
In the comment above it's said that "requires both local and module
resets to be deasserted"; if reset_control_status() returns 0 it means
that the reset line is deasserted.
So, it's pretty clear that the return value of reset_control_status()
was intended to be used instead of ti_sci_proc_get_status() return
value.
This could lead in an incorrect IPC-only mode detection if reset line is
asserted (so reset_control_status() return > 0) and c_state != 0 and
halted == 0.
In this case, the old code would have detected an IPC-only mode instead
of a mismatched mode.
Fixes: 1168af40b1ad ("remoteproc: k3-r5: Add support for IPC-only mode for all
R5Fs")
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 50e486bcfa10..39a47540c590 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -1144,6 +1144,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
u32 atcm_enable, btcm_enable, loczrama;
struct k3_r5_core *core0;
enum cluster_mode mode = cluster->mode;
+ int reset_ctrl_status;
int ret;
core0 = list_first_entry(&cluster->cores, struct k3_r5_core, elem);
@@ -1160,11 +1161,11 @@ static int k3_r5_rproc_configure_mode(struct
k3_r5_rproc *kproc)
r_state, c_state);
}
- ret = reset_control_status(core->reset);
- if (ret < 0) {
+ reset_ctrl_status = reset_control_status(core->reset);
+ if (reset_ctrl_status < 0) {
dev_err(cdev, "failed to get initial local reset status, ret =
%d\n",
- ret);
- return ret;
+ reset_ctrl_status);
+ return reset_ctrl_status;
}
/*
@@ -1199,7 +1200,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
* irrelevant if module reset is asserted (POR value has local reset
* deasserted), and is deemed as remoteproc mode
*/
- if (c_state && !ret && !halted) {
+ if (c_state && !reset_ctrl_status && !halted) {
dev_info(cdev, "configured R5F for IPC-only mode\n");
kproc->rproc->state = RPROC_DETACHED;
ret = 1;
@@ -1217,7 +1218,7 @@ static int k3_r5_rproc_configure_mode(struct k3_r5_rproc
*kproc)
ret = 0;
} else {
dev_err(cdev, "mismatched mode: local_reset = %s, module_reset
= %s, core_state = %s\n",
- !ret ? "deasserted" : "asserted",
+ !reset_ctrl_status ? "deasserted" : "asserted",
c_state ? "deasserted" : "asserted",
halted ? "halted" : "unhalted");
ret = -EINVAL;
[PATCH 4/4] remoteproc: k3-r5: support for graceful stop of remote cores
Introduce software IPC handshake between the K3-R5 remote proc driver
and the R5 MCU to gracefully stop/reset the remote core.
Upon a stop request, K3-R5 remote proc driver sends a RP_MBOX_SHUTDOWN
mailbox message to the remote R5 core.
The remote core is expected to:
- relinquish all the resources acquired through Device Manager (DM)
- disable its interrupts
- send back a mailbox acknowledgment RP_MBOX_SHUDOWN_ACK
- enter WFI state.
Meanwhile, the K3-R5 remote proc driver does:
- wait for the RP_MBOX_SHUTDOWN_ACK from the remote core
- wait for the remote proc to enter WFI state
- reset the remote core through device manager
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/omap_remoteproc.h | 9 +-
drivers/remoteproc/ti_k3_r5_remoteproc.c | 40
2 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/omap_remoteproc.h
b/drivers/remoteproc/omap_remoteproc.h
index 828e13256c02..c008f11fa2a4 100644
--- a/drivers/remoteproc/omap_remoteproc.h
+++ b/drivers/remoteproc/omap_remoteproc.h
@@ -42,6 +42,11 @@
* @RP_MBOX_SUSPEND_CANCEL: a cancel suspend response from a remote processor
* on a suspend request
*
+ * @RP_MBOX_SHUTDOWN: shutdown request for the remote processor
+ *
+ * @RP_MBOX_SHUTDOWN_ACK: successful response from remote processor for a
+ * shutdown request. The remote processor should be in WFI state short after.
+ *
* Introduce new message definitions if any here.
*
* @RP_MBOX_END_MSG: Indicates end of known/defined messages from remote core
@@ -59,7 +64,9 @@ enum omap_rp_mbox_messages {
RP_MBOX_SUSPEND_SYSTEM = 0xFF11,
RP_MBOX_SUSPEND_ACK = 0xFF12,
RP_MBOX_SUSPEND_CANCEL = 0xFF13,
- RP_MBOX_END_MSG = 0xFF14,
+ RP_MBOX_SHUTDOWN= 0xFF14,
+ RP_MBOX_SHUTDOWN_ACK= 0xFF15,
+ RP_MBOX_END_MSG = 0xFF16,
};
#endif /* _OMAP_RPMSG_H */
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index a2ead87952c7..918a15e1dd9a 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
@@ -172,8 +173,23 @@ struct k3_r5_rproc {
struct k3_r5_core *core;
struct k3_r5_mem *rmem;
int num_rmems;
+ struct completion shutdown_complete;
};
+/*
+ * This will return true if the remote core is in Wait For Interrupt state.
+ */
+static bool k3_r5_is_core_in_wfi(struct k3_r5_core *core)
+{
+ int ret;
+ u64 boot_vec;
+ u32 cfg, ctrl, stat;
+
+ ret = ti_sci_proc_get_status(core->tsp, &boot_vec, &cfg, &ctrl, &stat);
+
+ return !ret ? !!(stat & PROC_BOOT_STATUS_FLAG_R5_WFI) : false;
+}
+
/**
* k3_r5_rproc_mbox_callback() - inbound mailbox message handler
* @client: mailbox client pointer used for requesting the mailbox channel
@@ -209,6 +225,10 @@ static void k3_r5_rproc_mbox_callback(struct mbox_client
*client, void *data)
case RP_MBOX_ECHO_REPLY:
dev_info(dev, "received echo reply from %s\n", name);
break;
+ case RP_MBOX_SHUTDOWN_ACK:
+ dev_dbg(dev, "received shutdown_ack from %s\n", name);
+ complete(&kproc->shutdown_complete);
+ break;
default:
/* silently handle all other valid messages */
if (msg >= RP_MBOX_READY && msg < RP_MBOX_END_MSG)
@@ -634,6 +654,7 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_cluster *cluster = kproc->cluster;
struct device *dev = kproc->dev;
struct k3_r5_core *core1, *core = kproc->core;
+ bool wfi;
int ret;
@@ -650,6 +671,24 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
}
}
+ /* Send SHUTDOWN message to remote proc */
+ reinit_completion(&kproc->shutdown_complete);
+ ret = mbox_send_message(kproc->mbox, (void *)RP_MBOX_SHUTDOWN);
+ if (ret < 0) {
+ dev_err(dev, "Sending SHUTDOWN message failed: %d. Halting core
anyway.\n", ret);
+ } else {
+ ret = wait_for_completion_timeout(&kproc->shutdown_complete,
+ msecs_to_jiffies(1000));
+ if (ret == 0) {
+ dev_err(dev, "Timeout waiting SHUTDOWN_ACK message.
Halting core anyway.\n");
+ } else {
+ ret = readx_poll_timeout(k3_r5_is_core_in_wfi, core,
+wfi, wfi, 200, 2000);
+ if (ret)
+ dev_err(dev, "Timeout waiting for remote proc
to be in WFI state. Halting core anyway.\n"
[PATCH 3/4] remoteproc: k3-r5: k3_r5_rproc_stop: code reorder
In the next commit, a RP_MBOX_SHUTDOWN message will be sent in
k3_r5_rproc_stop() to the remote proc (in lockstep on not)
Thus, the sanity check "do not allow core 0 to stop before core 1"
should be moved at the beginning of the function so that the generic case
can be dealt with.
In order to have an easier patch to review, those actions are broke in
two patches:
- this patch: moving the sanity check at the beginning (No functional
change).
- next patch: doing the real job (sending shutdown messages to remote
procs before halting them).
Basically, we had:
- cluster_mode actions
- !cluster_mode sanity check
- !cluster_mode actions
And now:
- !cluster_mode sanity check
- cluster_mode actions
- !cluster_mode actions
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 24 ++--
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 1f18b08618c8..a2ead87952c7 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -636,16 +636,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
struct k3_r5_core *core1, *core = kproc->core;
int ret;
- /* halt all applicable cores */
- if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
- list_for_each_entry(core, &cluster->cores, elem) {
- ret = k3_r5_core_halt(core);
- if (ret) {
- core = list_prev_entry(core, elem);
- goto unroll_core_halt;
- }
- }
- } else {
+
+ if (cluster->mode != CLUSTER_MODE_LOCKSTEP) {
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
@@ -656,6 +648,18 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
ret = -EPERM;
goto out;
}
+ }
+
+ /* halt all applicable cores */
+ if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
+ list_for_each_entry(core, &cluster->cores, elem) {
+ ret = k3_r5_core_halt(core);
+ if (ret) {
+ core = list_prev_entry(core, elem);
+ goto unroll_core_halt;
+ }
+ }
+ } else {
ret = k3_r5_core_halt(core);
if (ret)
[PATCH 2/4] remoteproc: k3-r5: Introduce PM suspend/resume handlers
This patch adds the support for system suspend/resume to the ti_k3_R5
remoteproc driver.
In order to save maximum power, the approach here is to shutdown
completely the cores that were started by the kernel (i.e. those in
RUNNING state).
Those which were started before the kernel (in attached mode) will be
detached.
The pm_notifier mechanism is used here because the remote procs firmwares
have to be reloaded at resume, and thus the driver must have access to
the file system were the firmware is stored.
On suspend, the running remote procs are stopped, the attached remote
procs are detached and processor control released.
On resume, the reverse operation is done.
Based on work from: Hari Nagalla
Signed-off-by: Richard Genoud
---
drivers/remoteproc/ti_k3_r5_remoteproc.c | 123 ++-
1 file changed, 121 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/ti_k3_r5_remoteproc.c
b/drivers/remoteproc/ti_k3_r5_remoteproc.c
index 39a47540c590..1f18b08618c8 100644
--- a/drivers/remoteproc/ti_k3_r5_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_r5_remoteproc.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -112,6 +113,7 @@ struct k3_r5_cluster {
struct list_head cores;
wait_queue_head_t core_transition;
const struct k3_r5_soc_data *soc_data;
+ struct notifier_block pm_notifier;
};
/**
@@ -577,7 +579,8 @@ static int k3_r5_rproc_start(struct rproc *rproc)
/* do not allow core 1 to start before core 0 */
core0 = list_first_entry(&cluster->cores, struct k3_r5_core,
elem);
- if (core != core0 && core0->rproc->state == RPROC_OFFLINE) {
+ if (core != core0 && (core0->rproc->state == RPROC_OFFLINE ||
+ core0->rproc->state == RPROC_SUSPENDED)) {
dev_err(dev, "%s: can not start core 1 before core 0\n",
__func__);
ret = -EPERM;
@@ -646,7 +649,8 @@ static int k3_r5_rproc_stop(struct rproc *rproc)
/* do not allow core 0 to stop before core 1 */
core1 = list_last_entry(&cluster->cores, struct k3_r5_core,
elem);
- if (core != core1 && core1->rproc->state != RPROC_OFFLINE) {
+ if (core != core1 && core1->rproc->state != RPROC_OFFLINE &&
+ core1->rproc->state != RPROC_SUSPENDED) {
dev_err(dev, "%s: can not stop core 0 before core 1\n",
__func__);
ret = -EPERM;
@@ -1238,6 +1242,117 @@ static int k3_r5_rproc_configure_mode(struct
k3_r5_rproc *kproc)
return ret;
}
+static int k3_r5_rproc_suspend(struct k3_r5_rproc *kproc)
+{
+ unsigned int rproc_state = kproc->rproc->state;
+ int ret;
+
+ if (rproc_state != RPROC_RUNNING && rproc_state != RPROC_ATTACHED)
+ return 0;
+
+ if (rproc_state == RPROC_RUNNING)
+ ret = rproc_shutdown(kproc->rproc);
+ else
+ ret = rproc_detach(kproc->rproc);
+
+ if (ret) {
+ dev_err(kproc->dev, "Failed to %s rproc (%d)\n",
+ (rproc_state == RPROC_RUNNING) ? "shutdown" : "detach",
+ ret);
+ return ret;
+ }
+
+ kproc->rproc->state = RPROC_SUSPENDED;
+
+ return ret;
+}
+
+static int k3_r5_rproc_resume(struct k3_r5_rproc *kproc)
+{
+ int ret;
+
+ if (kproc->rproc->state != RPROC_SUSPENDED)
+ return 0;
+
+ ret = k3_r5_rproc_configure_mode(kproc);
+ if (ret < 0)
+ return -EBUSY;
+
+ /*
+* ret > 0 for IPC-only mode
+* ret == 0 for remote proc mode
+*/
+ if (ret == 0) {
+ /*
+* remote proc looses its configuration when powered off.
+* So, we have to configure it again on resume.
+*/
+ ret = k3_r5_rproc_configure(kproc);
+ if (ret < 0) {
+ dev_err(kproc->dev, "k3_r5_rproc_configure failed
(%d)\n", ret);
+ return -EBUSY;
+ }
+ }
+
+ return rproc_boot(kproc->rproc);
+}
+
+static int k3_r5_cluster_pm_notifier_call(struct notifier_block *bl,
+ unsigned long state, void *unused)
+{
+ struct k3_r5_cluster *cluster = container_of(bl, struct k3_r5_cluster,
+pm_notifier);
+ struct k3_r5_core *core;
+ int ret;
+
+ switch (state) {
+ case PM_HIBERNATION_PREPARE
[PATCH 0/4] remoteproc: k3-r5: Introduce suspend to ram support
This series enables the suspend to ram with R5F remote processors on TI K3 platform. The 1st patch is actually a fix, independent from the others The 2nd patch introduces the suspend/resume handlers. On suspend, the running rprocs will be stopped (or detached) and then re-loaded in resume. The logic behind this is: - shutdown the cores that Linux started to save power in suspend. - detach the cores that were started before Linux. Then, the 3rd and 4th patches introduce the graceful shutdown of remote procs. This will give them a chance to release resources and shut down in a civilized manner. Without this series, the suspend fails with: omap-mailbox 31f81000.mailbox: fifo 1 has unexpected unread messages omap-mailbox 31f81000.mailbox: PM: dpm_run_callback(): platform_pm_suspend returns -16 omap-mailbox 31f81000.mailbox: PM: platform_pm_suspend returned -16 after 16328 usecs omap-mailbox 31f81000.mailbox: PM: failed to suspend: error -16 Patches 2 and 4 are based on work from Hari Nagalla . @Hari, please feel free to add your Co-developed-by:/Signed-off-by: Tested on J7200X SoM Series is based on v6.10-rc4 Richard Genoud (4): remoteproc: k3-r5: Fix IPC-only mode detection remoteproc: k3-r5: Introduce PM suspend/resume handlers remoteproc: k3-r5: k3_r5_rproc_stop: code reorder remoteproc: k3-r5: support for graceful stop of remote cores drivers/remoteproc/omap_remoteproc.h | 9 +- drivers/remoteproc/ti_k3_r5_remoteproc.c | 196 +-- 2 files changed, 188 insertions(+), 17 deletions(-)
Re: [PATCH 2/4] tty: atmel_serial: convert tasklets to use new tasklet_setup() API
Le 17/08/2020 à 10:59, Allen Pais a écrit :
> From: Allen Pais
>
> In preparation for unconditionally passing the
> struct tasklet_struct pointer to all tasklet
> callbacks, switch to using the new tasklet_setup()
> and from_tasklet() to pass the tasklet pointer explicitly.
>
> Signed-off-by: Romain Perier
> Signed-off-by: Allen Pais
Acked-by: Richard Genoud
> ---
> drivers/tty/serial/atmel_serial.c | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index e43471b33710..a9c47f56e994 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1722,10 +1722,11 @@ static int atmel_prepare_rx_pdc(struct uart_port
> *port)
> /*
> * tasklet handling tty stuff outside the interrupt handler.
> */
> -static void atmel_tasklet_rx_func(unsigned long data)
> +static void atmel_tasklet_rx_func(struct tasklet_struct *t)
> {
> - struct uart_port *port = (struct uart_port *)data;
> - struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + struct atmel_uart_port *atmel_port = from_tasklet(atmel_port, t,
> + tasklet_rx);
> + struct uart_port *port = &atmel_port->uart;
>
> /* The interrupt handler does not take the lock */
> spin_lock(&port->lock);
> @@ -1733,10 +1734,11 @@ static void atmel_tasklet_rx_func(unsigned long data)
> spin_unlock(&port->lock);
> }
>
> -static void atmel_tasklet_tx_func(unsigned long data)
> +static void atmel_tasklet_tx_func(struct tasklet_struct *t)
> {
> - struct uart_port *port = (struct uart_port *)data;
> - struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + struct atmel_uart_port *atmel_port = from_tasklet(atmel_port, t,
> + tasklet_tx);
> + struct uart_port *port = &atmel_port->uart;
>
> /* The interrupt handler does not take the lock */
> spin_lock(&port->lock);
> @@ -1911,10 +1913,8 @@ static int atmel_startup(struct uart_port *port)
> }
>
> atomic_set(&atmel_port->tasklet_shutdown, 0);
> - tasklet_init(&atmel_port->tasklet_rx, atmel_tasklet_rx_func,
> - (unsigned long)port);
> - tasklet_init(&atmel_port->tasklet_tx, atmel_tasklet_tx_func,
> - (unsigned long)port);
> + tasklet_setup(&atmel_port->tasklet_rx, atmel_tasklet_rx_func);
> + tasklet_setup(&atmel_port->tasklet_tx, atmel_tasklet_tx_func);
>
> /*
>* Initialize DMA (if necessary)
>
Thanks !
Re: [PATCH] can: m_can_platform: fix m_can_runtime_suspend()
Hi Dan,
Le 08/06/2020 à 16:27, Dan Murphy a écrit :
Richard
On 6/8/20 4:43 AM, Richard Genoud wrote:
Since commit f524f829b75a ("can: m_can: Create a m_can platform
framework"), the can peripheral on STM32MP1 wasn't working anymore.
The reason was a bad copy/paste maneuver that added a call to
m_can_class_suspend() in m_can_runtime_suspend().
Are you sure it was a copy paste error?
Probably don't want to have an unfounded cause unless you know for
certain it was this.
I understand.
What makes me think it was a copy-paste error is that the primary goal
of the patch series "M_CAN Framework" was to introduce the tcan4x5x
driver into the kernel.
For that, the code has to be split into a re-usable code (m_can.c) and a
platform code m_can_platform.c
And finally, tcan4x5x.c can be added.
(I'm sure you already know that since you write the patch, it's just to
be sure that we are on the same page :))
So, when splitting the m_can code into m_can.c and m_can_platform.c,
there was no reason to change the behavior, even less reason to change
the behavior in m_can_platform.c, since the main target was tcan4x5x.
(And the behavior changed because the CAN peripheral on the STM32MP1 was
working before this patch, and not after).
So I went digging into that and I realized that before this patch,
runtime suspend function was in m_can.c:
static int __maybe_unused m_can_runtime_suspend(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
struct m_can_priv *priv = netdev_priv(ndev);
clk_disable_unprepare(priv->cclk);
clk_disable_unprepare(priv->hclk);
return 0;
}
And after, in m_can_platform.c:
static int __maybe_unused m_can_runtime_suspend(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
struct m_can_priv *mcan_class = netdev_priv(ndev);
m_can_class_suspend(dev);
clk_disable_unprepare(mcan_class->cclk);
clk_disable_unprepare(mcan_class->hclk);
return 0;
}
Same for runtime resume,
Before:
static int __maybe_unused m_can_runtime_resume(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
struct m_can_priv *priv = netdev_priv(ndev);
int err;
err = clk_prepare_enable(priv->hclk);
if (err)
return err;
err = clk_prepare_enable(priv->cclk);
if (err)
clk_disable_unprepare(priv->hclk);
return err;
}
After:
static int __maybe_unused m_can_runtime_resume(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
struct m_can_priv *mcan_class = netdev_priv(ndev);
int err;
err = clk_prepare_enable(mcan_class->hclk);
if (err)
return err;
err = clk_prepare_enable(mcan_class->cclk);
if (err)
clk_disable_unprepare(mcan_class->hclk);
m_can_class_resume(dev);
return err;
}
Now, the m_class_resume() call has been removed by commit 0704c5743694
("can: m_can_platform: remove unnecessary m_can_class_resume() call")
cf https://lkml.org/lkml/2019/11/19/965
Then only the m_can_class_suspend() call is left alone. If I remove it,
the stm32mp1 peripheral works as before the patch. (and the code is
symmetrical again :))
I read all the iterations I could find about this patch (see note 1),
and I didn't found any comment on the addition of
m_can_class_{resume,suspend}() calls.
But I found this in v3 cover letter:
"The m_can platform code will need to be updated as I have not tested
this code."
and in v3 1/4 comments:
"This patch set is working for the TCAN and at least boots on io-mapped
devices."
For me, that means that the code in m_can_platform.c was written with
this sentence in mind :
"I can test everything but this, so let's try not to break things in
there, keep the changes at a minimum"
And that was really the case for all the file, but the 2 calls to
m_can_class_{resume,suspend}().
So that's why I have a pretty good confidence in the fact that it was a
copy-paste error.
And, moreover, if m_can_class_suspend() is called, the CAN device is
stopped, and all interrupts are disabled (in m_can_stop()), so the
device can not wake-up by itself (and thus not working anymore).
All this make me think that maybe I should send a v2 of this patch with
a bigger commit message.
What do you think ?
Thanks !
Richard.
Dan
Note 1: patches v3 to v12 (missing v11)
https://lwn.net/ml/linux-kernel/2019073236.14329-1-dmur...@ti.com/
https://lore.kernel.org/patchwork/patch/1033094/
https://lore.kernel.org/patchwork/cover/1042441/
https://lore.kernel.org/patchwork/patch/1047220/
https://lore.kernel.org/patchwork/patch/1047980/
https://lkml.org/lkml/2019/3/12/362
https://lkml.org/lkml/2019/3/13/512
https://www.spinics.net/lists/netdev/msg557961.html
https://lore.kernel.org/patchwork/patch/1071894/
[PATCH] can: m_can_platform: fix m_can_runtime_suspend()
Since commit f524f829b75a ("can: m_can: Create a m_can platform
framework"), the can peripheral on STM32MP1 wasn't working anymore.
The reason was a bad copy/paste maneuver that added a call to
m_can_class_suspend() in m_can_runtime_suspend().
Tested on STM32MP157C-DK2 and emSBC-Argon
Fixes: f524f829b75a ("can: m_can: Create a m_can platform framework")
Signed-off-by: Richard Genoud
---
drivers/net/can/m_can/m_can_platform.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/can/m_can/m_can_platform.c
b/drivers/net/can/m_can/m_can_platform.c
index 38ea5e600fb8..e6d0cb9ee02f 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -144,8 +144,6 @@ static int __maybe_unused m_can_runtime_suspend(struct
device *dev)
struct net_device *ndev = dev_get_drvdata(dev);
struct m_can_classdev *mcan_class = netdev_priv(ndev);
- m_can_class_suspend(dev);
-
clk_disable_unprepare(mcan_class->cclk);
clk_disable_unprepare(mcan_class->hclk);
Re: [PATCH v6 2/2] tty: add rpmsg driver
Hi Arnaud,
Le 04/09/2019 à 15:09, Arnaud Pouliquen a écrit :
> This driver exposes a standard tty interface on top of the rpmsg
> framework through a rpmsg service.
>
> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
> per rpmsg endpoint.
>
> Signed-off-by: Arnaud Pouliquen
> Signed-off-by: Fabien Dessenne
> ---
> Documentation/serial/tty_rpmsg.rst | 45
> drivers/tty/Kconfig| 9 +
> drivers/tty/Makefile | 1 +
> drivers/tty/rpmsg_tty.c| 418
> +
> 4 files changed, 473 insertions(+)
> create mode 100644 Documentation/serial/tty_rpmsg.rst
> create mode 100644 drivers/tty/rpmsg_tty.c
>
> diff --git a/Documentation/serial/tty_rpmsg.rst
> b/Documentation/serial/tty_rpmsg.rst
> new file mode 100644
> index ..fc1d3fba73c5
> --- /dev/null
> +++ b/Documentation/serial/tty_rpmsg.rst
> @@ -0,0 +1,45 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=
> +The rpmsg TTY
> +=
> +
> +The rpmsg tty driver implements serial communication on the RPMsg bus to
> makes possible for user-space programs to send and receive rpmsg messages as
> a standard tty protocol.
> +
> +The remote processor can instantiate a new tty by requesting:
> +- a "rpmsg-tty-raw" RPMsg service, for TTY raw data support without flow
> control
> +- a "rpmsg-tty-ctrl" RPMSg service, for TTY support with flow control.
> +
> +Information related to the RPMsg and associated tty device is available in
> +/sys/bus/rpmsg/devices/.
> +
> +RPMsg TTY without control
> +-
> +
> +The default end point associated with the "rpmsg-tty-raw" service is directly
> +used for data exchange. No flow control is available.
> +
> +To be compliant with this driver, the remote firmware must create its data
> end point associated with the "rpmsg-tty-raw" service.
> +
> +RPMsg TTY with control
> +-
> +
> +The default end point associated with the "rpmsg-tty-ctrl" service is
> reserved for
> +the control. A second endpoint must be created for data exchange.
> +
> +The control channel is used to transmit to the remote processor the CTS
> status,
> +as well as the end point address for data transfer.
> +
> +To be compatible with this driver, the remote firmware must create or use
> its end point associated with "rpmsg-tty-ctrl" service, plus a second
> endpoint for the data flow.
> +On Linux rpmsg_tty probes, the data endpoint address and the CTS (set to
> disable)
> +is sent to the remote processor.
> +The remote processor has to respect following rules:
> +- It only transmits data when Linux remote cts is enable, otherwise message
> + could be lost.
> +- It can pause/resume reception by sending a control message (rely on CTS
> state).
> +
> +Control message structure:
> +struct rpmsg_tty_ctrl {
> + u8 cts; /* remote reception status */
> + u16 d_ept_addr; /* data endpoint address */
> +};
Correct me if I'm wrong, but I think this structure should be packed,
shouldn't it ?
It's working ok on the STM32MP127, between M4 and A7, but the alignment
may be not the same on another architecture ?
> diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
> index c7623f99ac0f..1046bf4aa709 100644
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -454,6 +454,15 @@ config VCC
> help
> Support for Sun logical domain consoles.
>
> +config RPMSG_TTY
> + tristate "RPMSG tty driver"
> + depends on RPMSG
> + help
> + Say y here to export rpmsg endpoints as tty devices, usually found
> + in /dev/ttyRPMSGx.
> + This makes it possible for user-space programs to send and receive
> + rpmsg messages as a standard tty protocol.
> +
> config LDISC_AUTOLOAD
> bool "Automatically load TTY Line Disciplines"
> default y
> diff --git a/drivers/tty/Makefile b/drivers/tty/Makefile
> index 020b1cd9294f..c2465e7ebc2a 100644
> --- a/drivers/tty/Makefile
> +++ b/drivers/tty/Makefile
> @@ -34,5 +34,6 @@ obj-$(CONFIG_PPC_EPAPR_HV_BYTECHAN) += ehv_bytechan.o
> obj-$(CONFIG_GOLDFISH_TTY) += goldfish.o
> obj-$(CONFIG_MIPS_EJTAG_FDC_TTY) += mips_ejtag_fdc.o
> obj-$(CONFIG_VCC)+= vcc.o
> +obj-$(CONFIG_RPMSG_TTY) += rpmsg_tty.o
>
> obj-y += ipwireless/
> diff --git a/drivers/tty/rpmsg_tty.c b/drivers/tty/rpmsg_tty.c
> new file mode 100644
> index ..3e4d0e1a6663
> --- /dev/null
> +++ b/drivers/tty/rpmsg_tty.c
> @@ -0,0 +1,418 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) STMicroelectronics 2019 - All Rights Reserved
> + * Authors: Arnaud Pouliquen for
> STMicroelectronics.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define MAX_TTY_RPMSG32
> +
> +#define TTY_CH_NAME_RAW "rpmsg-tty-raw"
> +#define TTY_CH_NAME_WITH_CTS "rpmsg-tty-ctrl"
> +
> +static DEFINE_IDR(tty_idr); /* tty instance id */
[PATCH v2] tty/serial: atmel: remove unneeded atmel_get_lines_status function
Since commit 18dfef9c7f87 ("serial: atmel: convert to irq handling
provided mctrl-gpio"), the GPIOs interrupts are handled by
mctrl_gpio_irq_handle().
So, atmel_get_lines_status() can be completely killed and replaced by :
atmel_uart_readl(port, ATMEL_US_CSR);
Suggested-by: Uwe Kleine-König
Acked-by: Uwe Kleine-König
Signed-off-by: Richard Genoud
---
drivers/tty/serial/atmel_serial.c | 48 ++-
1 file changed, 2 insertions(+), 46 deletions(-)
Changes from v1:
- point out the right commit (thx Uwe)
- add Suggested-by/Acked-by
diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index 9a54c9e6d36e..a8dc8af83f39 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -294,50 +294,6 @@ static void atmel_tasklet_schedule(struct atmel_uart_port
*atmel_port,
tasklet_schedule(t);
}
-static unsigned int atmel_get_lines_status(struct uart_port *port)
-{
- struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
- unsigned int status, ret = 0;
-
- status = atmel_uart_readl(port, ATMEL_US_CSR);
-
- mctrl_gpio_get(atmel_port->gpios, &ret);
-
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_CTS))) {
- if (ret & TIOCM_CTS)
- status &= ~ATMEL_US_CTS;
- else
- status |= ATMEL_US_CTS;
- }
-
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_DSR))) {
- if (ret & TIOCM_DSR)
- status &= ~ATMEL_US_DSR;
- else
- status |= ATMEL_US_DSR;
- }
-
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_RI))) {
- if (ret & TIOCM_RI)
- status &= ~ATMEL_US_RI;
- else
- status |= ATMEL_US_RI;
- }
-
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_DCD))) {
- if (ret & TIOCM_CD)
- status &= ~ATMEL_US_DCD;
- else
- status |= ATMEL_US_DCD;
- }
-
- return status;
-}
-
/* Enable or disable the rs485 support */
static int atmel_config_rs485(struct uart_port *port,
struct serial_rs485 *rs485conf)
@@ -1453,7 +1409,7 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
spin_lock(&atmel_port->lock_suspended);
do {
- status = atmel_get_lines_status(port);
+ status = atmel_uart_readl(port, ATMEL_US_CSR);
mask = atmel_uart_readl(port, ATMEL_US_IMR);
pending = status & mask;
if (!pending)
@@ -2002,7 +1958,7 @@ static int atmel_startup(struct uart_port *port)
}
/* Save current CSR for comparison in atmel_tasklet_func() */
- atmel_port->irq_status_prev = atmel_get_lines_status(port);
+ atmel_port->irq_status_prev = atmel_uart_readl(port, ATMEL_US_CSR);
/*
* Finally, enable the serial port
[PATCH] tty/serial: atmel: remove unneeded atmel_get_lines_status function
Since commit ce59e48fdbad ("serial: mctrl_gpio: implement interrupt
handling"), the GPIOs interrupts are handled by mctrl_gpio_irq_handle().
So, atmel_get_lines_status() can be completely killed and replaced by :
atmel_uart_readl(port, ATMEL_US_CSR);
Signed-off-by: Richard Genoud
---
drivers/tty/serial/atmel_serial.c | 48 ++-
1 file changed, 2 insertions(+), 46 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index 9a54c9e6d36e..a8dc8af83f39 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -294,50 +294,6 @@ static void atmel_tasklet_schedule(struct atmel_uart_port
*atmel_port,
tasklet_schedule(t);
}
-static unsigned int atmel_get_lines_status(struct uart_port *port)
-{
- struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
- unsigned int status, ret = 0;
-
- status = atmel_uart_readl(port, ATMEL_US_CSR);
-
- mctrl_gpio_get(atmel_port->gpios, &ret);
-
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_CTS))) {
- if (ret & TIOCM_CTS)
- status &= ~ATMEL_US_CTS;
- else
- status |= ATMEL_US_CTS;
- }
-
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_DSR))) {
- if (ret & TIOCM_DSR)
- status &= ~ATMEL_US_DSR;
- else
- status |= ATMEL_US_DSR;
- }
-
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_RI))) {
- if (ret & TIOCM_RI)
- status &= ~ATMEL_US_RI;
- else
- status |= ATMEL_US_RI;
- }
-
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_DCD))) {
- if (ret & TIOCM_CD)
- status &= ~ATMEL_US_DCD;
- else
- status |= ATMEL_US_DCD;
- }
-
- return status;
-}
-
/* Enable or disable the rs485 support */
static int atmel_config_rs485(struct uart_port *port,
struct serial_rs485 *rs485conf)
@@ -1453,7 +1409,7 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
spin_lock(&atmel_port->lock_suspended);
do {
- status = atmel_get_lines_status(port);
+ status = atmel_uart_readl(port, ATMEL_US_CSR);
mask = atmel_uart_readl(port, ATMEL_US_IMR);
pending = status & mask;
if (!pending)
@@ -2002,7 +1958,7 @@ static int atmel_startup(struct uart_port *port)
}
/* Save current CSR for comparison in atmel_tasklet_func() */
- atmel_port->irq_status_prev = atmel_get_lines_status(port);
+ atmel_port->irq_status_prev = atmel_uart_readl(port, ATMEL_US_CSR);
/*
* Finally, enable the serial port
--
2.19.2
Re: [PATCH v2 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped
Le 19/03/2019 à 14:20, Razvan Stefanescu a écrit :
> In half-duplex operation, RX should be started after TX completes.
>
> If DMA is used, there is a case when the DMA transfer completes but the
> TX FIFO is not emptied, so the RX cannot be restarted just yet.
>
> Use a boolean variable to store this state and rearm TX interrupt mask
> to be signaled again that the transfer finished. In interrupt transmit
> handler this variable is used to start RX. A warning message is generated
> if RX is activated before TX fifo is cleared.
>
> Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable
> RX after TX is done")
> Signed-off-by: Razvan Stefanescu
Acked-by: Richard Genoud
NB: backport on kernel older than 4.20 will fail because of the iso7816
variables fidi_min/fidi_max.
> ---
> Changelog:
> v2:
> - start RX and display warning in case of error
> - add fix info
>
> drivers/tty/serial/atmel_serial.c | 25 ++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index b4b89a16a41b..5b2f859c327c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -166,6 +166,8 @@ struct atmel_uart_port {
> unsigned intpending_status;
> spinlock_t lock_suspended;
>
> + boolhd_start_rx;/* can start RX during
> half-duplex operation */
> +
> /* ISO7816 */
> unsigned intfidi_min;
> unsigned intfidi_max;
> @@ -933,8 +935,13 @@ static void atmel_complete_tx_dma(void *arg)
> if (!uart_circ_empty(xmit))
> atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
> else if (atmel_uart_is_half_duplex(port)) {
> - /* DMA done, stop TX, start RX for RS485 */
> - atmel_start_rx(port);
> + /*
> + * DMA done, re-enable TXEMPTY and signal that we can stop
> + * TX and start RX for RS485
> + */
> + atmel_port->hd_start_rx = true;
> + atmel_uart_writel(port, ATMEL_US_IER,
> + atmel_port->tx_done_mask);
> }
>
> spin_unlock_irqrestore(&port->lock, flags);
> @@ -1378,9 +1385,20 @@ atmel_handle_transmit(struct uart_port *port, unsigned
> int pending)
> struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>
> if (pending & atmel_port->tx_done_mask) {
> - /* Either PDC or interrupt transmission */
> atmel_uart_writel(port, ATMEL_US_IDR,
> atmel_port->tx_done_mask);
> +
> + /* Start RX if flag was set and FIFO is empty */
> + if (atmel_port->hd_start_rx) {
> + if (!(atmel_uart_readl(port, ATMEL_US_CSR)
> + & ATMEL_US_TXEMPTY))
> + dev_warn(port->dev, "Should start RX, but TX
> fifo is not empty\n");
> +
> + atmel_port->hd_start_rx = false;
> + atmel_start_rx(port);
> + return;
> + }
> +
> atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
> }
> }
>
Re: [PATCH v2 1/2] tty/serial: atmel: Add is_half_duplex helper
Le 19/03/2019 à 14:20, Razvan Stefanescu a écrit :
> Use a helper function to check that a port needs to use half duplex
> communication, replacing several occurrences of multi-line bit checking.
>
> Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable
> RX after TX is done")
> Signed-off-by: Razvan Stefanescu
Acked-by: Richard Genoud
NB: backport on kernel older than 4.20 will fail because of the
SER_ISO7816_ENABLED flag.
> ---
> Changelog:
> v2:
> - remove extra check
> - add fix info
>
> drivers/tty/serial/atmel_serial.c | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 05147fe24343..b4b89a16a41b 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -231,6 +231,13 @@ static inline void atmel_uart_write_char(struct
> uart_port *port, u8 value)
> __raw_writeb(value, port->membase + ATMEL_US_THR);
> }
>
> +static inline int atmel_uart_is_half_duplex(struct uart_port *port)
> +{
> + return ((port->rs485.flags & SER_RS485_ENABLED) &&
> + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> + (port->iso7816.flags & SER_ISO7816_ENABLED);
> +}
> +
> #ifdef CONFIG_SERIAL_ATMEL_PDC
> static bool atmel_use_pdc_rx(struct uart_port *port)
> {
> @@ -608,10 +615,9 @@ static void atmel_stop_tx(struct uart_port *port)
> /* Disable interrupts */
> atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
>
> - if (((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> - port->iso7816.flags & SER_ISO7816_ENABLED)
> + if (atmel_uart_is_half_duplex(port))
> atmel_start_rx(port);
> +
> }
>
> /*
> @@ -628,9 +634,7 @@ static void atmel_start_tx(struct uart_port *port)
> return;
>
> if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
> - if (((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> - port->iso7816.flags & SER_ISO7816_ENABLED)
> + if (atmel_uart_is_half_duplex(port))
> atmel_stop_rx(port);
>
> if (atmel_use_pdc_tx(port))
> @@ -928,9 +932,7 @@ static void atmel_complete_tx_dma(void *arg)
>*/
> if (!uart_circ_empty(xmit))
> atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
> - else if (((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> - port->iso7816.flags & SER_ISO7816_ENABLED) {
> + else if (atmel_uart_is_half_duplex(port)) {
> /* DMA done, stop TX, start RX for RS485 */
> atmel_start_rx(port);
> }
> @@ -1508,9 +1510,7 @@ static void atmel_tx_pdc(struct uart_port *port)
> atmel_uart_writel(port, ATMEL_US_IER,
> atmel_port->tx_done_mask);
> } else {
> - if (((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> - port->iso7816.flags & SER_ISO7816_ENABLED) {
> + if (atmel_uart_is_half_duplex(port)) {
> /* DMA done, stop TX, start RX for RS485 */
> atmel_start_rx(port);
> }
>
Re: [PATCH 2/2] tty/serial: atmel: RS485 HD w/DMA: enable RX after TX is stopped
[ adding Gil Weber in Cc: ]
Le 15/03/2019 à 10:23, Razvan Stefanescu a écrit :
> In half-duplex operation, RX should be started after TX completes.
>
> If DMA is used, there is a case when the DMA transfer completes but the
> TX FIFO is not emptied, so the RX cannot be restarted just yet.
>
> Use a boolean variable to store this state and rearm TX interrupt mask
> to be signaled again that the transfer finished. In interrupt transmit
> handler this variable is used to start RX.
I was sure I've already seen something like that.
Gil, could you give it a test ?
you can add :
Cc: sta...@vger.kernel.org
Fixes: b389f173aaa1 ("tty/serial: atmel: RS485 half duplex w/DMA: enable RX
after TX is done")
Since patch 1/2 is needed for this one, I think you should add also
Cc:stable / Fixes: to the previous patch.
> Signed-off-by: Razvan Stefanescu
> ---
> drivers/tty/serial/atmel_serial.c | 25 ++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index a6577b1c4461..b0141dcbbb61 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -166,6 +166,8 @@ struct atmel_uart_port {
> unsigned intpending_status;
> spinlock_t lock_suspended;
>
> + boolhd_start_rx;/* can start RX during
> half-duplex operation */
> +
> /* ISO7816 */
> unsigned intfidi_min;
> unsigned intfidi_max;
> @@ -934,8 +936,13 @@ static void atmel_complete_tx_dma(void *arg)
> if (!uart_circ_empty(xmit))
> atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
> else if (atmel_uart_is_half_duplex(port)) {
> - /* DMA done, stop TX, start RX for RS485 */
> - atmel_start_rx(port);
> + /*
> + * DMA done, re-enable TXEMPTY and signal that we can stop
> + * TX and start RX for RS485
> + */
> + atmel_port->hd_start_rx = true;
> + atmel_uart_writel(port, ATMEL_US_IER,
> + atmel_port->tx_done_mask);
> }
>
> spin_unlock_irqrestore(&port->lock, flags);
> @@ -1379,9 +1386,21 @@ atmel_handle_transmit(struct uart_port *port, unsigned
> int pending)
> struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
>
> if (pending & atmel_port->tx_done_mask) {
> - /* Either PDC or interrupt transmission */
> atmel_uart_writel(port, ATMEL_US_IDR,
> atmel_port->tx_done_mask);
> +
> + /* Start RX if flag was set and FIFO is empty */
> + if (atmel_port->hd_start_rx) {
> + if (atmel_uart_readl(port, ATMEL_US_CSR)
> + & ATMEL_US_TXEMPTY) {
> + atmel_port->hd_start_rx = false;
> + atmel_start_rx(port);
> + } else {
> + dev_warn(port->dev, "Should start RX, but TX
> fifo is not empty\n");
What will happen in this case ?
> + }
> + return;
> + }
> +
> atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
> }
> }
>
Thanks !
Re: [PATCH 1/2] tty/serial: atmel: Add is_half_duplex helper
[ adding Gil Weber in Cc: ]
Le 15/03/2019 à 10:23, Razvan Stefanescu a écrit :
> Use a helper function to check that a port needs to use half duplex
> communication, replacing several occurrences of multi-line bit checking.
>
> Signed-off-by: Razvan Stefanescu
> ---
> drivers/tty/serial/atmel_serial.c | 27 ++-
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 05147fe24343..a6577b1c4461 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -231,6 +231,13 @@ static inline void atmel_uart_write_char(struct
> uart_port *port, u8 value)
> __raw_writeb(value, port->membase + ATMEL_US_THR);
> }
>
> +static inline int atmel_uart_is_half_duplex(struct uart_port *port)
> +{
> + return ((port->rs485.flags & SER_RS485_ENABLED) &&
> + !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> + (port->iso7816.flags & SER_ISO7816_ENABLED);
> +}
> +
> #ifdef CONFIG_SERIAL_ATMEL_PDC
> static bool atmel_use_pdc_rx(struct uart_port *port)
> {
> @@ -608,10 +615,10 @@ static void atmel_stop_tx(struct uart_port *port)
> /* Disable interrupts */
> atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
>
> - if (((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> - port->iso7816.flags & SER_ISO7816_ENABLED)
> - atmel_start_rx(port);
> + if (atmel_uart_is_half_duplex(port))
> + if (!atomic_read(&atmel_port->tasklet_shutdown))
This check wasn't there before.
If it's really needed, please insert it in another patch and explain why
it's needed.
> + atmel_start_rx(port);
> +
> }
>
> /*
> @@ -628,9 +635,7 @@ static void atmel_start_tx(struct uart_port *port)
> return;
>
> if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
> - if (((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> - port->iso7816.flags & SER_ISO7816_ENABLED)
> + if (atmel_uart_is_half_duplex(port))
> atmel_stop_rx(port);
>
> if (atmel_use_pdc_tx(port))
> @@ -928,9 +933,7 @@ static void atmel_complete_tx_dma(void *arg)
>*/
> if (!uart_circ_empty(xmit))
> atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_tx);
> - else if (((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> - port->iso7816.flags & SER_ISO7816_ENABLED) {
> + else if (atmel_uart_is_half_duplex(port)) {
> /* DMA done, stop TX, start RX for RS485 */
> atmel_start_rx(port);
> }
> @@ -1508,9 +1511,7 @@ static void atmel_tx_pdc(struct uart_port *port)
> atmel_uart_writel(port, ATMEL_US_IER,
> atmel_port->tx_done_mask);
> } else {
> - if (((port->rs485.flags & SER_RS485_ENABLED) &&
> - !(port->rs485.flags & SER_RS485_RX_DURING_TX)) ||
> - port->iso7816.flags & SER_ISO7816_ENABLED) {
> + if (atmel_uart_is_half_duplex(port)) {
> /* DMA done, stop TX, start RX for RS485 */
> atmel_start_rx(port);
> }
>
Re: [PATCH] tty: atmel_serial: fix a NULL pointer dereference
Le 15/03/2019 à 18:16, Kangjie Lu a écrit :
> In case dmaengine_prep_dma_cyclic fails, the fix returns a proper
> error code to avoid NULL pointer dereference.
>
> Signed-off-by: Kangjie Lu
> Fixes: 34df42f59a60 ("serial: at91: add rx dma support")
Acked-by: Richard Genoud
>
> ---
> V2: simplified the patch as suggested by
> Richard Genoud
> ---
> drivers/tty/serial/atmel_serial.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 05147fe24343..41b728d223d1 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1288,6 +1288,10 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>sg_dma_len(&atmel_port->sg_rx)/2,
>DMA_DEV_TO_MEM,
>DMA_PREP_INTERRUPT);
> + if (!desc) {
> + dev_err(port->dev, "Preparing DMA cyclic failed\n");
> + goto chan_err;
> + }
> desc->callback = atmel_complete_rx_dma;
> desc->callback_param = port;
> atmel_port->desc_rx = desc;
>
Thanks !
Richard
Re: [PATCH v2] tty: atmel_serial: fix a NULL pointer dereference
Le 15/03/2019 à 08:27, Kangjie Lu a écrit :
> Fixes: 34df42f59a60 ("serial: at91: add rx dma support")
The Fixes: tag should be just bellow the Signenf-off-by: tag
>
> In case dmaengine_prep_dma_cyclic fails, the fix returns a proper
> error code to avoid NULL pointer dereference.
>
> Signed-off-by: Kangjie Lu
^^^
here
>
> ---
> V2: simplified the patch as suggested by
> Richard Genoud
> ---
> drivers/tty/serial/atmel_serial.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 05147fe24343..41b728d223d1 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1288,6 +1288,10 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>sg_dma_len(&atmel_port->sg_rx)/2,
>DMA_DEV_TO_MEM,
>DMA_PREP_INTERRUPT);
> + if (!desc) {
> + dev_err(port->dev, "Preparing DMA cyclic failed\n");
> + goto chan_err;
> + }
> desc->callback = atmel_complete_rx_dma;
> desc->callback_param = port;
> atmel_port->desc_rx = desc;
>
Thanks !
Richard.
Re: [PATCH] tty: atmel_serial: fix a NULL pointer dereference
Hi,
Good catch !
Le 14/03/2019 à 08:17, Kangjie Lu a écrit :
> In case dmaengine_prep_dma_cyclic fails, the fix return a proper
> error code to avoid NULL pointer dereference.
>
you could add:
Fixes: 34df42f59a60 ("serial: at91: add rx dma support")
So that -stable branches get this.
> Signed-off-by: Kangjie Lu
> ---
> drivers/tty/serial/atmel_serial.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 05147fe24343..cf560d05008c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1237,8 +1237,10 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> dma_cap_set(DMA_CYCLIC, mask);
>
> atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
> - if (atmel_port->chan_rx == NULL)
> + if (atmel_port->chan_rx == NULL) {
> + ret = -EINVAL;
> goto chan_err;
> + }
> dev_info(port->dev, "using %s for rx DMA transfers\n",
> dma_chan_name(atmel_port->chan_rx));
>
> @@ -1257,6 +1259,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>
> if (!nent) {
> dev_dbg(port->dev, "need to release resource of dma\n");
> + ret = -EINVAL;
> goto chan_err;
> } else {
> dev_dbg(port->dev, "%s: mapped %d@%p to %pad\n", __func__,
> @@ -1288,6 +1291,11 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
>sg_dma_len(&atmel_port->sg_rx)/2,
>DMA_DEV_TO_MEM,
>DMA_PREP_INTERRUPT);
> + if (!desc) {
> + dev_err(port->dev, "Preparing DMA cyclic failed\n");
> + ret = -ENOMEM;
IMHO, we don't really know why dmaengine_prep_dma_cyclic() failed, it
could be because it's already in use, or bad value, or...
(and anyway, we just check if the return value is < 0 in atmel _startup.)
Is there a specific reason you choose -ENOMEM ?
If not, maybe keeping this patch smaller with a simple dev_err()+goto
here would be a better choice.
> + goto chan_err;
> + }
> desc->callback = atmel_complete_rx_dma;
> desc->callback_param = port;
> atmel_port->desc_rx = desc;
> @@ -1300,7 +1308,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> atmel_port->use_dma_rx = 0;
> if (atmel_port->chan_rx)
> atmel_release_rx_dma(port);
> - return -EINVAL;
> + return ret;
> }
>
> static void atmel_uart_timer_callback(struct timer_list *t)
>
Thanks !
Richard.
[PATCH] dmaengine: at_hdmac: fix memory leak in at_dma_xlate()
The leak was found when opening/closing a serial port a great number of
time, increasing kmalloc-32 in slabinfo.
Each time the port was opened, dma_request_slave_channel() was called.
Then, in at_dma_xlate(), atslave was allocated with devm_kzalloc() and
never freed. (Well, it was free at module unload, but that's not what we
want).
So, here, kzalloc is more suited for the job since it has to be freed in
atc_free_chan_resources().
Cc: sta...@vger.kernel.org
Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
Reported-by: Mario Forner
Suggested-by: Alexandre Belloni
Acked-by: Alexandre Belloni
Signed-off-by: Richard Genoud
---
drivers/dma/at_hdmac.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 7cbac6e8c113..1b7f0ca0d5cd 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1641,6 +1641,12 @@ static void atc_free_chan_resources(struct dma_chan
*chan)
atchan->descs_allocated = 0;
atchan->status = 0;
+ /*
+* Free atslave allocated in at_dma_xlate()
+*/
+ kfree(chan->private);
+ chan->private = NULL;
+
dev_vdbg(chan2dev(chan), "free_chan_resources: done\n");
}
@@ -1675,7 +1681,7 @@ static struct dma_chan *at_dma_xlate(struct
of_phandle_args *dma_spec,
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
- atslave = devm_kzalloc(&dmac_pdev->dev, sizeof(*atslave), GFP_KERNEL);
+ atslave = kzalloc(sizeof(*atslave), GFP_KERNEL);
if (!atslave)
return NULL;
[PATCH] dmaengine: at_hdmac: fix module unloading
of_dma_controller_free() was not called on module onloading.
This lead to a soft lockup:
watchdog: BUG: soft lockup - CPU#0 stuck for 23s!
Modules linked in: at_hdmac [last unloaded: at_hdmac]
when of_dma_request_slave_channel() tried to call ofdma->of_dma_xlate().
Cc: sta...@vger.kernel.org
Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
Signed-off-by: Richard Genoud
---
drivers/dma/at_hdmac.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 1b7f0ca0d5cd..01d936c9fe89 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -2006,6 +2006,8 @@ static int at_dma_remove(struct platform_device *pdev)
struct resource *io;
at_dma_off(atdma);
+ if (pdev->dev.of_node)
+ of_dma_controller_free(pdev->dev.of_node);
dma_async_device_unregister(&atdma->dma_common);
dma_pool_destroy(atdma->memset_pool);
Re: DMA: atmel_serial: Opening and closing the serial device repeatedly causes kmalloc-32 slab leak
Le 27/11/2018 à 10:58, Alexandre Belloni a écrit :
> Hello Richard,
>
> On 27/11/2018 10:51:13+0100, richard.gen...@gmail.com wrote:
>> Hi all,
>>
>> I reproduced the memory leak on my board (at91sam9g35-cm) with a 4.20-rc3.
>>
>> It triggered an OOM after a couple of hours running a code like this:
>> #include
>> #include
>> #include
>> #include
>>
>>
>> int main(int argc, char **argv)
>> {
>> int fd;
>> do {
>> fd = open("/dev/ttyS1", O_RDONLY);
>> close(fd);
>> } while (true);
>> return 0;
>> }
>>
>> As Mario pointed out, this only happens when atmel,use-dma-{r,t}x are
>> used in the device-tree.
>>
>> Adding:
>> CONFIG_DEBUG_SLAB=y
>> CONFIG_DEBUG_SLAB_LEAK=y
>> Doesn't show anything suspect in /proc/slab_allocators
>>
>> From what I found until now, it's something done in :
>> dma_request_slave_channel();
>> that leaks kmalloc-32
>> Mabe I missed something, but it seems that everything DMA related is
>> deallocated in atmel_release_{tx,rx}_dma().
>>
>> Is this ringing a bell ?
>>
>
> Yes, this is known issue and it has yet to be worked on.
>
After a talk on freenode, Alex found the problem.
A patch is on its way.
Thanks !
DMA: atmel_serial: Opening and closing the serial device repeatedly causes kmalloc-32 slab leak
[re-sending with Vinod's correct email address. Sorry for the noise. ]
[re-sending the bug report to the lists]
Le 16/11/2018 à 17:04, Mario Forner a écrit :
> Problem:
> When I open and close the serial device /dev/ttyS4 in a loop
> the amount of kmalloc-32 slabs increases slowly but steadily without limit.
>
> The serial device is configured in acme-aria.dts to use DMA.
>
> If DMA is disabled, the amount of kmalloc32-slabs remains constant
> over several hours, just fluctuating slightly.
>
> The serial device is accessed by atmel_serial.c which is evident from
> the drivers kernel log output.
>
> The bug was noticed on a device which had been running over several weeks and
> has accumulated ~86MB of unrelaimable kmalloc-32 slabs by now. Example:
>
> root@master1083:~# slabtop -o | head -10
> Active / Total Objects (% used): 2704880 / 2716124 (99.6%)
> Active / Total Slabs (% used) : 23150 / 23150 (100.0%)
> Active / Total Caches (% used) : 57 / 76 (75.0%)
> Active / Total Size (% used) : 88893.62K / 89964.32K (98.8%)
> Minimum / Average / Maximum Object : 0.02K / 0.03K / 4096.00K
>
> OBJS ACTIVE USE OBJ SIZE SLABS OBJ/SLAB CACHE SIZE NAME
> 2674804 2673840 99%0.03K 21571 124 86284K kmalloc-32
> 7200 7104 98%0.08K144 50 576K kernfs_node_cache
> 6930 5370 77%0.13K231 30 924K dentry
>
> root@master1083:~# uptime
> 15:12:55 up 93 days, 16:54, 1 user, load average: 1.59, 2.31, 2.42
>
> Keywords: atmel, serial, kernel, leak, memory, dma, slab, kmalloc
>
> Kernel information:
>
> Kernel version:
> Linux version 4.2.6 (mario@linux-7rm0) (gcc version 4.9.3 (crosstool-NG ) )
> #115 Fri Nov 16 11:05:22 CET 2018
> Linux version 4.9.124 (mario@linux-7rm0) (gcc version 4.9.3 (crosstool-NG ) )
> #16 Fri Nov 16 13:54:25 CET 2018
>
> Most recent Kernel version which did not have the bug: unknown
>
> How to reproduce the bug:
>
> The kmalloc-32 slab count should be monitored every few minutes with "slabtop
> -o".
> The leak can be triggered by running the following script, given DMA has
> been enabled for ttyS4:
>
> #!/usr/bin/env python
> import serial
> import time
>
> while True:
> try:
> with serial.Serial(port = '/dev/ttyS4'):
> pass
> except Exception as e:
> print e
> finally:
> time.sleep(0.5)
> # end script
>
> Environment:
> Software:
> Linux master 4.2.6 #114 Fri Nov 16 10:14:30 CET 2018 armv5tejl GNU/Linux
>
> Binutils2.25
> Util-linux 2.25.2
> Mount 2.25.2
> Module-init-tools 18
> E2fsprogs 1.42.12
> Linux C Library 2.19
> Dynamic linker (ldd)2.19
> Linux C++ Library 6.0.20
> Procps 3.3.9
> Net-tools 1.60
> Sh-utils8.23
> Udev215
> Modules Loaded iptable_nat option usb_wwan usbserial
>
> Processor information:
> # cat /proc/cpuinfo
> processor : 0
> model name : ARM926EJ-S rev 5 (v5l)
> BogoMIPS: 198.76
> Features: swp half thumb fastmult edsp java
> CPU implementer : 0x41
> CPU architecture: 5TEJ
> CPU variant : 0x0
> CPU part: 0x926
> CPU revision: 5
>
> Hardware: Atmel AT91SAM9
> Revision:
> Serial :
>
> Module information:
> # cat /proc/modules
> iptable_nat 1720 0 - Live 0xbf0a5000
> option 28780 0 - Live 0xbf03c000
> usb_wwan 6876 1 option, Live 0xbf024000
> usbserial 23392 2 option,usb_wwan, Live 0xbf00
>
> Loaded drivers:
> # cat /proc/ioports
> # cat /proc/iomem
> 0030-00307fff : 30.sram
> 0060-006f : /ahb/ohci@0060
> 0070-007f : /ahb/ehci@0070
> 2000-2fff : System RAM
> 20008000-20577287 : Kernel code
> 205a8000-205f02b7 : Kernel data
> f000-f0ff : /ahb/apb/spi@f000
> f8008000-f80080ff : /ahb/apb/timer@f8008000
> f800c000-f800c0ff : /ahb/apb/timer@f800c000
> f801-f80100ff : /ahb/apb/i2c@f801
> f8014000-f80140ff : /ahb/apb/i2c@f8014000
> f801c000-f801c1ff : atmel_serial
> f802-f80201ff : atmel_serial
> f8024000-f80241ff : atmel_serial
> f8028000-f80281ff : atmel_serial
> f802c000-f802c0ff : /ahb/apb/ethernet@f802c000
> f8034000-f80342ff : /ahb/apb/pwm@f8034000
> f804c000-f804c0ff : /ahb/apb/adc@f804c000
> ec00-edff : at_hdmac
> ee00-efff : at_hdmac
> f200-f3ff : atmel_serial
> f400-f5ff : /ahb/apb/pinctrl@f400/gpio@f400
> f600-f7ff : /ahb/apb/pinctrl@f400/gpio@f600
> f800-f9ff : /ahb/apb/pinctrl@f400/gpio@f800
> fa00-fbff : /ahb/apb/pinctrl@f400/gpio@fa00
> fe10-fe1f : /ahb/apb/shdwc@fe10
>
> Other Information:
> The serial device ttyS4 is configuered inside .dts file by
>
> usart3: serial@f8028000 {
> status = "okay";
> compatible = "atmel,at91sam9260-usart";
> reg = <0xf8028000 0x
DMA: atmel_serial: Opening and closing the serial device repeatedly causes kmalloc-32 slab leak
[re-sending the bug report to the lists]
Le 16/11/2018 à 17:04, Mario Forner a écrit :
> Problem:
> When I open and close the serial device /dev/ttyS4 in a loop
> the amount of kmalloc-32 slabs increases slowly but steadily without limit.
>
> The serial device is configured in acme-aria.dts to use DMA.
>
> If DMA is disabled, the amount of kmalloc32-slabs remains constant
> over several hours, just fluctuating slightly.
>
> The serial device is accessed by atmel_serial.c which is evident from
> the drivers kernel log output.
>
> The bug was noticed on a device which had been running over several weeks and
> has accumulated ~86MB of unrelaimable kmalloc-32 slabs by now. Example:
>
> root@master1083:~# slabtop -o | head -10
> Active / Total Objects (% used): 2704880 / 2716124 (99.6%)
> Active / Total Slabs (% used) : 23150 / 23150 (100.0%)
> Active / Total Caches (% used) : 57 / 76 (75.0%)
> Active / Total Size (% used) : 88893.62K / 89964.32K (98.8%)
> Minimum / Average / Maximum Object : 0.02K / 0.03K / 4096.00K
>
> OBJS ACTIVE USE OBJ SIZE SLABS OBJ/SLAB CACHE SIZE NAME
> 2674804 2673840 99%0.03K 21571 124 86284K kmalloc-32
> 7200 7104 98%0.08K144 50 576K kernfs_node_cache
> 6930 5370 77%0.13K231 30 924K dentry
>
> root@master1083:~# uptime
> 15:12:55 up 93 days, 16:54, 1 user, load average: 1.59, 2.31, 2.42
>
> Keywords: atmel, serial, kernel, leak, memory, dma, slab, kmalloc
>
> Kernel information:
>
> Kernel version:
> Linux version 4.2.6 (mario@linux-7rm0) (gcc version 4.9.3 (crosstool-NG ) )
> #115 Fri Nov 16 11:05:22 CET 2018
> Linux version 4.9.124 (mario@linux-7rm0) (gcc version 4.9.3 (crosstool-NG ) )
> #16 Fri Nov 16 13:54:25 CET 2018
>
> Most recent Kernel version which did not have the bug: unknown
>
> How to reproduce the bug:
>
> The kmalloc-32 slab count should be monitored every few minutes with "slabtop
> -o".
> The leak can be triggered by running the following script, given DMA has
> been enabled for ttyS4:
>
> #!/usr/bin/env python
> import serial
> import time
>
> while True:
> try:
> with serial.Serial(port = '/dev/ttyS4'):
> pass
> except Exception as e:
> print e
> finally:
> time.sleep(0.5)
> # end script
>
> Environment:
> Software:
> Linux master 4.2.6 #114 Fri Nov 16 10:14:30 CET 2018 armv5tejl GNU/Linux
>
> Binutils2.25
> Util-linux 2.25.2
> Mount 2.25.2
> Module-init-tools 18
> E2fsprogs 1.42.12
> Linux C Library 2.19
> Dynamic linker (ldd)2.19
> Linux C++ Library 6.0.20
> Procps 3.3.9
> Net-tools 1.60
> Sh-utils8.23
> Udev215
> Modules Loaded iptable_nat option usb_wwan usbserial
>
> Processor information:
> # cat /proc/cpuinfo
> processor : 0
> model name : ARM926EJ-S rev 5 (v5l)
> BogoMIPS: 198.76
> Features: swp half thumb fastmult edsp java
> CPU implementer : 0x41
> CPU architecture: 5TEJ
> CPU variant : 0x0
> CPU part: 0x926
> CPU revision: 5
>
> Hardware: Atmel AT91SAM9
> Revision:
> Serial :
>
> Module information:
> # cat /proc/modules
> iptable_nat 1720 0 - Live 0xbf0a5000
> option 28780 0 - Live 0xbf03c000
> usb_wwan 6876 1 option, Live 0xbf024000
> usbserial 23392 2 option,usb_wwan, Live 0xbf00
>
> Loaded drivers:
> # cat /proc/ioports
> # cat /proc/iomem
> 0030-00307fff : 30.sram
> 0060-006f : /ahb/ohci@0060
> 0070-007f : /ahb/ehci@0070
> 2000-2fff : System RAM
> 20008000-20577287 : Kernel code
> 205a8000-205f02b7 : Kernel data
> f000-f0ff : /ahb/apb/spi@f000
> f8008000-f80080ff : /ahb/apb/timer@f8008000
> f800c000-f800c0ff : /ahb/apb/timer@f800c000
> f801-f80100ff : /ahb/apb/i2c@f801
> f8014000-f80140ff : /ahb/apb/i2c@f8014000
> f801c000-f801c1ff : atmel_serial
> f802-f80201ff : atmel_serial
> f8024000-f80241ff : atmel_serial
> f8028000-f80281ff : atmel_serial
> f802c000-f802c0ff : /ahb/apb/ethernet@f802c000
> f8034000-f80342ff : /ahb/apb/pwm@f8034000
> f804c000-f804c0ff : /ahb/apb/adc@f804c000
> ec00-edff : at_hdmac
> ee00-efff : at_hdmac
> f200-f3ff : atmel_serial
> f400-f5ff : /ahb/apb/pinctrl@f400/gpio@f400
> f600-f7ff : /ahb/apb/pinctrl@f400/gpio@f600
> f800-f9ff : /ahb/apb/pinctrl@f400/gpio@f800
> fa00-fbff : /ahb/apb/pinctrl@f400/gpio@fa00
> fe10-fe1f : /ahb/apb/shdwc@fe10
>
> Other Information:
> The serial device ttyS4 is configuered inside .dts file by
>
> usart3: serial@f8028000 {
> status = "okay";
> compatible = "atmel,at91sam9260-usart";
> reg = <0xf8028000 0x200>;
> interrupts = <8 IRQ_TYPE_LEVEL_HIGH 5>;
> pinctrl-names
Re: [PATCH v4 2/2] tty/serial: atmel: add ISO7816 support
Hi, Ludovic, On 06/09/2018 15:42, Ludovic Desroches wrote: > From: Nicolas Ferre > > When mode is set in atmel_config_iso7816() we backup last RS232 mode > for coming back to this mode if requested. > Also allow setup of T=0 and T=1 parameter and basic support in set_termios > function as well. > > Signed-off-by: Nicolas Ferre > [ludovic.desroc...@microchip.com: rebase, add check on fidi ratio, checkpatch > fixes] > Signed-off-by: Ludovic Desroches This seems ok for me. Acked-by: Richard Genoud Thanks !
Re: [PATCH v3 2/2] tty/serial: atmel: add ISO7816 support
[added Nicolas back in the thread, he was removed somehow]
Hi Ludovic !
On 05/09/2018 14:43, Ludovic Desroches wrote:
> Hi Richard,
>
> On Thu, Aug 09, 2018 at 01:30:35PM +0200, Ludovic Desroches wrote:
>> Hi Richard,
>>
>> On Thu, Aug 09, 2018 at 10:47:17AM +0200, Richard Genoud wrote:
>>> Hi !
>>>
>>> On 07/08/2018 15:00, Ludovic Desroches wrote:
>>>> From: Nicolas Ferre
>>>>
>>>> When mode is set in atmel_config_iso7816() we backup last RS232 mode
>>>> for coming back to this mode if requested.
>>>> Also allow setup of T=0 and T=1 parameter and basic support in set_termios
>>>> function as well.
>>>>
>>>> Signed-off-by: Nicolas Ferre
>>>> [ludovic.desroc...@microchip.com: rebase, add check on fidi ratio,
>>>> checkpatch fixes]
>>>> Signed-off-by: Ludovic Desroches
>>>> ---
>>>> drivers/tty/serial/atmel_serial.c | 211
>>>> +++---
>>>> drivers/tty/serial/atmel_serial.h | 6 +-
>>>> 2 files changed, 201 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/drivers/tty/serial/atmel_serial.c
>>>> b/drivers/tty/serial/atmel_serial.c
>>>> index 8e4428725848..4a7ec44b0ace 100644
>>>> --- a/drivers/tty/serial/atmel_serial.c
>>>> +++ b/drivers/tty/serial/atmel_serial.c
>>
>> [...]
>>
>>>> #if defined(CONFIG_OF)
>>>> +static struct atmel_uart_pdata at91rm9200_pdata = {
>>>> + .fidi_min = 1,
>>>> + .fidi_max = 2047,
>>>> +};
>>>> +
>>>> +static struct atmel_uart_pdata at91sam9260_pdata = {
>>>> + .fidi_min = 1,
>>>> + .fidi_max = 2047,
>>>> +};
>>>> +
>>>> +static struct atmel_uart_pdata sama5d3_pdata = {
>>>> + .fidi_min = 3,
>>>> + .fidi_max = 65535,
>>> Are you sure this is for sama5d3 ?
>>> From the datasheets I have, 65535 is for sama5d4/sama5d2
>>
>> I checked it and I missed it. What a pity... In fact, it's a bit more
>> tricky since the min value for d3 is 3 and no longer 1.
>>
>>> And also, you'll have to s/atmel,at91sam9260-usart/atmel,sama5d2-usart/g
>>> in sama5d{2,4}.dtsi
>>>
>>
>> Yes, I planed to send it later but I can add those patches within this
>> set of patches.
>>
>>> But I wonder if it could be detected via ATMEL_US_VERSION instead ?
>>>
>>
>> I have not checked, I tend to prefer the compatible string for this kind
>> of thing. But as we already use the version number, I can investigate
>> this solution if it's the one you prefer.
>>
>
> ping about this question still in suspend in order to prepare a new version.
Well, if we use the compatible string for this, we will have to add :
- atmel,sama5d2-usart
- atmel,sama5d3-usart
- (maybe others ?)
to the already existing :
- atmel,at91sam9260-usart
- atmel,at91rm9200-usart
just for setting different limits on the fidi register.
IMHO, it seems a bit overkill. Moreover, the ATMEL_US_VERSION has
already been read, so...
But if you think adding compatible strings is a better/cleaner solution, just
convince me ! :)
Re: [PATCH RESEND 2/2] gpio: mvebu: Allow to use non-default PWM counter
Hi, On 06/08/2018 15:52, Andrew Lunn wrote: > On Mon, Aug 06, 2018 at 10:29:16AM +0800, Aditya Prayoga wrote: >> On multiple PWM lines, if the other PWM counter is unused, allocate it >> to next PWM request. The priority would be: >> 1. Default counter assigned to the bank >> 2. Unused counter that is assigned to other bank >> 3. Fallback to default counter >> >> For example on second bank there are three PWM request, first one would >> use default counter (counter B), second one would try to use counter A, >> and the third one would use counter B. > > Hi Aditya > > There are only two PWM counters for all the GPIO lines. So you cannot > support 3 PWM requests. You have to enforce a maximum of two PWMs. > > When i implemented this PWM code, i only needed one PWM. So it took > the easy option. GPIO bank 0 uses counter A, GPIO bank1 uses counter > B. For the hardware you have, this is not sufficient, so you need to > generalise this. Any PWM can use any counter, whatever is available > when the PWM is requested. > > Rather than have a linked list of PWM, i think it would be better to > have a static array of two mvebu_pwm structures. Index 0 uses counter > A, index 1 uses counter B. You can then keep with the concept of > pwm->pgiod != NULL means the counter is in use. The request() call can > then find an unused PWM, set pwm->gpiod, and point mvchip->mvpwm to > one of the two static instances. > > Andrew > I'm not sure that the logic: 1. Default counter assigned to the bank 2. Unused counter that is assigned to other bank 3. Fallback to default counter is the best one. I gave the code a try, and I've been a little confused. I declared: - fan1 as gpio1 22 - fan2 as gpio1 11 - fan3 as gpio0 22 and I did: echo 10 > hwmon1/pwm1 # ok echo 100 > hwmon2/pwm1 # still ok echo 200 > hwmon3/pwm1 # hey !! my fan2 is now at 200 (I can see it with the scope) # but cat hwmon2/pwm1 100 # okay, I want my fan2 back, So I turn off fan3: echo 0 > hwmon3/pwm1 # fan2 and fan3 are stopped echo 100 > hwmon2/pwm1 # not working.. fan2 is still at 0 on the scope but: cat hwmon2/pwm1 100 IMHO, I would either: - allow only 2 pwm and no more (but that's a pity) - allow lots of fans, but once 2 different speeds are set, return EINVAL for another different speed (even if it's on another bank) That way, we'll be able to switch on/off 1, 2, 3 or more fans, as long as they have the same speed. I'll give an example : echo 10 > hwmon1/pwm1 # ok echo 100 > hwmon2/pwm1 # still ok echo 200 > hwmon3/pwm1 # returns EINVAL echo 10 > hwmon3/pwm1 # ok The headache will come when we want to change the speed... echo 50 > hwmon3/pwm1 # should this change the hwmon1 as well or return EINVAL ? I'd say that it changes hwmon1 as well, as if hwmon1 and hwmon3 were tied. But, If I then do : echo 100 > hwmon3/pwm1 # fan2 was already at 100 What should happen ? Do fan1, fan2 and fan3 be set to 100 ? Or fan1 stay at 10 and fan3 at 100 ? (in this case, fan3 won't be tied to fan1 anymore, but to fan2) Hum... I don't know if anyone followed me on this... Anyway, I think I convinced myself that only allowing 2 pwm is less confusing than anything else :) Richard.
Re: [PATCH v3 2/2] tty/serial: atmel: add ISO7816 support
Hi !
On 07/08/2018 15:00, Ludovic Desroches wrote:
> From: Nicolas Ferre
>
> When mode is set in atmel_config_iso7816() we backup last RS232 mode
> for coming back to this mode if requested.
> Also allow setup of T=0 and T=1 parameter and basic support in set_termios
> function as well.
>
> Signed-off-by: Nicolas Ferre
> [ludovic.desroc...@microchip.com: rebase, add check on fidi ratio, checkpatch
> fixes]
> Signed-off-by: Ludovic Desroches
> ---
> drivers/tty/serial/atmel_serial.c | 211
> +++---
> drivers/tty/serial/atmel_serial.h | 6 +-
> 2 files changed, 201 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 8e4428725848..4a7ec44b0ace 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -34,6 +34,7 @@
> #include
> #include
>
> +#include
> #include
> #include
>
> @@ -83,6 +84,11 @@ static void atmel_stop_rx(struct uart_port *port);
>
> #define ATMEL_ISR_PASS_LIMIT 256
>
> +struct atmel_uart_pdata {
> + unsigned intfidi_min;
> + unsigned intfidi_max;
> +};
> +
> struct atmel_dma_buffer {
> unsigned char *buf;
> dma_addr_t dma_addr;
> @@ -114,6 +120,7 @@ struct atmel_uart_char {
> */
> struct atmel_uart_port {
> struct uart_portuart; /* uart */
> + const struct atmel_uart_pdata *pdata; /* SoC specific parameters */
> struct clk *clk; /* uart clock */
> int may_wakeup; /* cached value of
> device_may_wakeup for times we need to disable it */
> u32 backup_imr; /* IMR saved during suspend */
> @@ -147,6 +154,8 @@ struct atmel_uart_port {
> struct circ_buf rx_ring;
>
> struct mctrl_gpios *gpios;
> + u32 backup_mode;/* MR saved during iso7816
> operations */
> + u32 backup_brgr;/* BRGR saved during iso7816
> operations */
> unsigned inttx_done_mask;
> u32 fifo_size;
> u32 rts_high;
> @@ -192,10 +201,34 @@ static struct console atmel_console;
> #endif
>
> #if defined(CONFIG_OF)
> +static struct atmel_uart_pdata at91rm9200_pdata = {
> + .fidi_min = 1,
> + .fidi_max = 2047,
> +};
> +
> +static struct atmel_uart_pdata at91sam9260_pdata = {
> + .fidi_min = 1,
> + .fidi_max = 2047,
> +};
> +
> +static struct atmel_uart_pdata sama5d3_pdata = {
> + .fidi_min = 3,
> + .fidi_max = 65535,
Are you sure this is for sama5d3 ?
>From the datasheets I have, 65535 is for sama5d4/sama5d2
And also, you'll have to s/atmel,at91sam9260-usart/atmel,sama5d2-usart/g
in sama5d{2,4}.dtsi
But I wonder if it could be detected via ATMEL_US_VERSION instead ?
> +};
> +
> static const struct of_device_id atmel_serial_dt_ids[] = {
> - { .compatible = "atmel,at91rm9200-usart" },
> - { .compatible = "atmel,at91sam9260-usart" },
> - { /* sentinel */ }
> + {
> + .compatible = "atmel,at91rm9200-usart",
> + .data = &at91rm9200_pdata,
> + }, {
> + .compatible = "atmel,at91sam9260-usart",
> + .data = &at91sam9260_pdata,
> + }, {
> + .compatible = "atmel,sama5d3-usart",
> + .data = &sama5d3_pdata,
> + }, {
> + /* sentinel */
> + }
> };
> #endif
>
> @@ -362,6 +395,127 @@ static int atmel_config_rs485(struct uart_port *port,
> return 0;
> }
>
> +static unsigned int atmel_calc_cd(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + unsigned int cd;
> + u64 mck_rate;
> +
> + mck_rate = (u64)clk_get_rate(atmel_port->clk);
> + do_div(mck_rate, iso7816conf->clk);
> + cd = mck_rate;
> + return cd;
> +}
> +
> +static unsigned int atmel_calc_fidi(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + u64 fidi = 0;
> +
> + if (iso7816conf->sc_fi && iso7816conf->sc_di) {
> + fidi = (u64)iso7816conf->sc_fi;
> + do_div(fidi, iso7816conf->sc_di);
> + }
> + return (u32)fidi;
> +}
> +
> +/* Enable or disable the iso7816 support */
> +/* Called with interrupts disabled */
> +static int atmel_config_iso7816(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + unsigned int mode;
> + unsigned int cd, fidi;
> + int ret = 0;
> +
> + /* Disable interrupts */
> + atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> +
> + mode = atmel_uart_readl(port, ATMEL_US_MR);
> +
> + if (iso7816conf->flags & SER_ISO7816_ENABLED) {
> + mode &=
Re: [PATCH 0/2] gpio: mvebu: Add support for multiple PWM lines
On 03/08/2018 11:42, Richard Genoud wrote: > On 03/08/2018 11:36, Gregory CLEMENT wrote: >> Hi Linus and Adita >> >> On dim., juil. 29 2018, Linus Walleij wrote: >> >>> Hoping for some review from Gergory, Ralph or Richard who all seem >>> to use this driver! >> >> Would it be possible to resend the series adding me in CC? I would >> like to comment the patches, but unfortunately it seems that I was in >> none of the mailing list where the series had been sent. >> >> I found the patches on patchwork, and started to have a look on it for >> example, in the patch "gpio: mvebu: Add support for multiple PWM lines >> per GPIO chip", I wonder why the id is stored as it was not used at all. >> >> Having the patch inlined in an email would male the review easier. > Same for me :) > Sorry, for the noise, but please use my gmail address. Thanks ! Richard
Re: [PATCH 0/2] gpio: mvebu: Add support for multiple PWM lines
On 03/08/2018 11:36, Gregory CLEMENT wrote: > Hi Linus and Adita > > On dim., juil. 29 2018, Linus Walleij wrote: > >> Hoping for some review from Gergory, Ralph or Richard who all seem >> to use this driver! > > Would it be possible to resend the series adding me in CC? I would > like to comment the patches, but unfortunately it seems that I was in > none of the mailing list where the series had been sent. > > I found the patches on patchwork, and started to have a look on it for > example, in the patch "gpio: mvebu: Add support for multiple PWM lines > per GPIO chip", I wonder why the id is stored as it was not used at all. > > Having the patch inlined in an email would male the review easier. Same for me :)
Re: [PATCH v2 2/2] tty/serial: atmel: add ISO7816 support
Hi Ludovic,
On 19/07/2018 10:47, Ludovic Desroches wrote:
> From: Nicolas Ferre
>
> When mode is set in atmel_config_iso7816() we backup last RS232 mode
> for coming back to this mode if requested.
> Also allow setup of T=0 and T=1 parameter and basic support in set_termios
> function as well.
> Report NACK and ITER errors in irq handler.
>
> Signed-off-by: Nicolas Ferre
> Signed-off-by: Ludovic Desroches
> ---
> drivers/tty/serial/atmel_serial.c | 170
> +++---
> drivers/tty/serial/atmel_serial.h | 3 +-
> 2 files changed, 162 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 8e4428725848..cec958f1e7d4 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -34,6 +34,7 @@
> #include
> #include
>
> +#include
> #include
> #include
>
> @@ -147,6 +148,8 @@ struct atmel_uart_port {
> struct circ_buf rx_ring;
>
> struct mctrl_gpios *gpios;
> + u32 backup_mode;/* MR saved during iso7816
> operations */
> + u32 backup_brgr;/* BRGR saved during iso7816
> operations */
> unsigned inttx_done_mask;
> u32 fifo_size;
> u32 rts_high;
> @@ -362,6 +365,132 @@ static int atmel_config_rs485(struct uart_port *port,
> return 0;
> }
>
> +static unsigned int atmel_calc_cd(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + unsigned int cd;
> + u64 mck_rate;
> +
> + mck_rate = (u64)clk_get_rate(atmel_port->clk);
> + do_div(mck_rate, iso7816conf->clk);
> + cd = mck_rate;
> + return cd;
> +}
> +
> +static unsigned int atmel_calc_fidi(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + u64 fidi = 0;
> +
> + if (iso7816conf->sc_fi && iso7816conf->sc_di) {
> + fidi = (u64)iso7816conf->sc_fi;
> + do_div(fidi, iso7816conf->sc_di);
> + }
> + return (u32)fidi;
> +}
> +
> +/* Enable or disable the iso7816 support */
> +/* Called with interrupts disabled */
> +static int atmel_config_iso7816(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + unsigned int mode, t;
> + unsigned int cd, fidi;
> + int ret = 0;
> +
> + /* Disable RX and TX */
> + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RXDIS | ATMEL_US_TXDIS);
> + /* Disable interrupts */
> + atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> +
> + mode = atmel_uart_readl(port, ATMEL_US_MR);
> +
> + if (iso7816conf->flags & SER_ISO7816_ENABLED) {
> + mode &= ~ATMEL_US_USMODE;
> +
> + if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> + == SER_ISO7816_T(0)) {
> + mode |= ATMEL_US_USMODE_ISO7816_T0;
> + t = 0;
> + } else if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> + == SER_ISO7816_T(1)) {
> + mode |= ATMEL_US_USMODE_ISO7816_T1;
> + t = 1;
> + } else {
> + dev_warn(port->dev, "ISO7816 Type not supported.
> Resetting\n");
> + memset(iso7816conf, 0, sizeof(struct serial_iso7816));
> + goto err_out;
> + }
> +
> + dev_dbg(port->dev, "Setting USART to ISO7816 mode T%d\n", t);
> +
> + mode &= ~(ATMEL_US_USCLKS | ATMEL_US_CHRL
> + | ATMEL_US_NBSTOP | ATMEL_US_PAR);
This could be merged in the mode &= line above.
> +
> + /* NACK configuration */
> + if ((iso7816conf->flags & SER_ISO7816_T_PARAM)
> + == SER_ISO7816_T(0))
> + mode |= ATMEL_US_DSNACK;
> + else
> + mode |= ATMEL_US_INACK;
This could be also part of the if () above.
> + /* select mck clock, and output */
> + mode |= ATMEL_US_USCLKS_MCK | ATMEL_US_CLKO;
> + /* set parity for normal/inverse mode + max iterations */
> + mode |= ATMEL_US_PAR_EVEN | ATMEL_US_NBSTOP_1 | (3 << 24);
Is this really needed ?
In the documentation, I found:
"The configuration is 8 data bits, even parity and 1 or 2 stop bits,
regardless of the values programmed in the CHRL, MODE9, PAR and CHMODE
fields."
And, for MAX_ITERATIONS, could you add a macro instead of (x << 24) ?
(ATMEL_US_MAX_ITER mask is already defined).
And why 3 ? Should the user-space be allowed to control the max
automatic iteration ? or is it more like a "same-value-for-every-one"
thing ?
Re: [PATCH v6 6/6] tty/serial: atmel: change the driver to work under at91-usart mfd
port->iotype= UPIO_MEM;
> port->flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP;
> port->ops = &atmel_pops;
> port->fifosize = 1;
> port->dev = &pdev->dev;
> - port->mapbase = pdev->resource[0].start;
> - port->irq = pdev->resource[1].start;
> + port->mapbase = mpdev->resource[0].start;
> + port->irq = mpdev->resource[1].start;
> port->rs485_config = atmel_config_rs485;
> - port->membase = NULL;
> + port->membase = NULL;
>
> memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
>
> /* for console, the clock could already be configured */
> if (!atmel_port->clk) {
> - atmel_port->clk = clk_get(&pdev->dev, "usart");
> + atmel_port->clk = clk_get(&mpdev->dev, "usart");
> if (IS_ERR(atmel_port->clk)) {
> ret = PTR_ERR(atmel_port->clk);
> atmel_port->clk = NULL;
> @@ -2694,13 +2696,22 @@ static void atmel_serial_probe_fifos(struct
> atmel_uart_port *atmel_port,
> static int atmel_serial_probe(struct platform_device *pdev)
> {
> struct atmel_uart_port *atmel_port;
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *np = pdev->dev.parent->of_node;
> void *data;
> int ret = -ENODEV;
> bool rs485_enabled;
>
> BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>
> + /*
> + * In device tree is no node with "atmel,at91rm9200-usart-serial"
I think you meant :
In device tree *there* is no...
With that,
Acked-by: Richard Genoud
> + * as compatible string. This driver is probed by at91-usart mfd driver
> + * which is just a wrapper over the atmel_serial driver and
> + * spi-at91-usart driver. All attributes needed by this driver are
> + * found in of_node of parent.
> + */
> + pdev->dev.of_node = np;
> +
> ret = of_alias_get_id(np, "serial");
> if (ret < 0)
> /* port id not found in platform data nor device-tree aliases:
> @@ -2835,6 +2846,7 @@ static int atmel_serial_remove(struct platform_device
> *pdev)
>
> clk_put(atmel_port->clk);
> atmel_port->clk = NULL;
> + pdev->dev.of_node = NULL;
>
> return ret;
> }
> @@ -2845,7 +2857,7 @@ static struct platform_driver atmel_serial_driver = {
> .suspend= atmel_serial_suspend,
> .resume = atmel_serial_resume,
> .driver = {
> - .name = "atmel_usart",
> + .name = "atmel_usart_serial",
> .of_match_table = of_match_ptr(atmel_serial_dt_ids),
> },
> };
>
Thanks !
Richard.
Re: [PATCH v5 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
Typo in the subject:
changed->change
On 04/06/2018 18:59, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
>
> Signed-off-by: Radu Pirea
> ---
> drivers/tty/serial/Kconfig| 1 +
> drivers/tty/serial/atmel_serial.c | 41 ++-
> 2 files changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
> depends on ARCH_AT91 || COMPILE_TEST
> select SERIAL_CORE
> select SERIAL_MCTRL_GPIO if GPIOLIB
> + select MFD_AT91_USART
> help
> This enables the driver for the on-chip UARTs of the Atmel
> AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..5c74e03396ef 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>
> #if defined(CONFIG_OF)
> static const struct of_device_id atmel_serial_dt_ids[] = {
> - { .compatible = "atmel,at91rm9200-usart" },
> - { .compatible = "atmel,at91sam9260-usart" },
> + { .compatible = "atmel,at91rm9200-usart-serial" },
> + { .compatible = "atmel,at91sam9260-usart-serial" },
Sorry, I didn't catch that before, but we can drop
"atmel,at91sam9260-usart-serial" don't we ?
Only "atmel,at91rm9200-usart-serial" is used in the MFD driver.
> { /* sentinel */ }
> };
> #endif
> @@ -915,6 +915,7 @@ static void atmel_tx_dma(struct uart_port *port)
> static int atmel_prepare_tx_dma(struct uart_port *port)
> {
> struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + struct device *mfd_dev = port->dev->parent;
> dma_cap_mask_t mask;
> struct dma_slave_config config;
> int ret, nent;
> @@ -922,7 +923,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
> dma_cap_zero(mask);
> dma_cap_set(DMA_SLAVE, mask);
>
> - atmel_port->chan_tx = dma_request_slave_channel(port->dev, "tx");
> + atmel_port->chan_tx = dma_request_slave_channel(mfd_dev, "tx");
> if (atmel_port->chan_tx == NULL)
> goto chan_err;
> dev_info(port->dev, "using %s for tx DMA transfers\n",
> @@ -1093,6 +1094,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
> static int atmel_prepare_rx_dma(struct uart_port *port)
> {
> struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + struct device *mfd_dev = port->dev->parent;
> struct dma_async_tx_descriptor *desc;
> dma_cap_mask_t mask;
> struct dma_slave_config config;
> @@ -1104,7 +1106,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> dma_cap_zero(mask);
> dma_cap_set(DMA_CYCLIC, mask);
>
> - atmel_port->chan_rx = dma_request_slave_channel(port->dev, "rx");
> + atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
> if (atmel_port->chan_rx == NULL)
> goto chan_err;
> dev_info(port->dev, "using %s for rx DMA transfers\n",
> @@ -1631,7 +1633,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
> static void atmel_init_property(struct atmel_uart_port *atmel_port,
> struct platform_device *pdev)
> {
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *np = pdev->dev.parent->of_node;
I think this is not needed anymore (cf atmel_probe())
>
> /* DMA/PDC usage specification */
> if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -,8 +2224,8 @@ static const char *atmel_type(struct uart_port *port)
> */
> static void atmel_release_port(struct uart_port *port)
> {
> - struct platform_device *pdev = to_platform_device(port->dev);
> - int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> + struct platform_device *mpdev = to_platform_device(port->dev->parent);
> + int size = resource_size(mpdev->resource);
>
> release_mem_region(port->mapbase, size);
>
> @@ -2238,8 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
> */
> static int atmel_request_port(struct uart_port *port)
> {
> - struct platform_device *pdev = to_platform_device(port->dev);
> - int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> + struct platform_device *mpdev = to_platform_device(port->dev->parent);
> + int size = resource_size(mpdev->resource);
>
> if (!request_mem_region(port->mapbase, size, "atmel_serial"))
> return -EBUSY;
> @@ -2341,27 +2343,28 @@ static int atmel_init_port(struct atmel_uart_port
> *atmel_port,
> {
> int ret;
> struct uart_port *port = &atmel_port->uart;
> + struct platform_device *mpdev = to_platform_device(pdev->dev.parent);
Re: [PATCH v4 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
On 25/05/2018 19:19, Radu Pirea wrote: > This patch modifies the place where resources and device tree properties > are searched. > > Signed-off-by: Radu Pirea > --- > drivers/tty/serial/Kconfig| 1 + > drivers/tty/serial/atmel_serial.c | 40 +-- > 2 files changed, 23 insertions(+), 18 deletions(-) the stdout-path property of the chosen node is still broken in this verion. if : stdout-path = "serial0:115200n8"; is set in the DTS, the console output should go on serial0 (aka dbgu) cf Documentation/devicetree/bindings/chosen.txt With this patch applied, this is not the case anymore. Adding console=ttyS0,115200 in the chosen node is not the solution here. regards, Richard.
Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
On 25/05/2018 14:17, Radu Pirea wrote: > > > On 05/15/2018 04:14 PM, Richard Genoud wrote: >> On 15/05/2018 14:47, Radu Pirea wrote: >>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote: >>>> After your patch, the DMA is not selected anymore: >>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not >>>> available, switch to pio >>>> instead of: >>>> atmel_usart f200.serial: using dma1chan2 for tx DMA transfers >>>> >>> Fixed. >>>> And the kernel doesn't log anymore on the serial console, despite the >>>> loglevel=8 >>>> (after reverting this series, the kernel logs reappears on the serial >>>> console) >>>> >>> Which serial are you using as console? >> f200.serial (sam9g35-cm) >> ( stdout-path = "serial0:115200n8"; in the DTS ) >> >> With this series applied, all the kernel log goes on the screen. >> Without, it goes on the serial debug. >> > I tested again with archlinux arm and poky-linux4sam release as distros > and kernel log goes on the serial debug. Can you give me more details > like cmdline? I used kernel 4.17-rc6 at91_dt_defconfig at91sam9g35ek.dtb Kernel command line: root=/dev/mtdblock1 rw rootfstype=ubifs ubi.mtd=1 root=ubi0:rootfs ( the one from the DTS ) Detailed instructions: git checkout v4.17-rc6 ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 at91_dt_defconfig ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/ >From uboot: tftpboot 0x20007FC0 uImage tftpboot 0x2640 at91sam9g35ek.dtb bootm 0x20007FC0 - 0x2640 [ I see the logs on the serial debug ] git am \[PATCH\ v3\ [1-6]* ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/ >From uboot: tftpboot 0x20007FC0 uImage tftpboot 0x2640 at91sam9g35ek.dtb bootm 0x20007FC0 - 0x2640 [ I don't see the logs on the serial debug anymore ] >>>> (tests done on sam9g35) >>>> >>> I will consider the rest of suggestions. >>>> regards, >>>> Richard >>
Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
On 15/05/2018 14:47, Radu Pirea wrote: > On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote: >> After your patch, the DMA is not selected anymore: >> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not >> available, switch to pio >> instead of: >> atmel_usart f200.serial: using dma1chan2 for tx DMA transfers >> > Fixed. >> And the kernel doesn't log anymore on the serial console, despite the >> loglevel=8 >> (after reverting this series, the kernel logs reappears on the serial >> console) >> > Which serial are you using as console? f200.serial (sam9g35-cm) ( stdout-path = "serial0:115200n8"; in the DTS ) With this series applied, all the kernel log goes on the screen. Without, it goes on the serial debug. >> (tests done on sam9g35) >> > I will consider the rest of suggestions. >> regards, >> Richard
Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
On 14/05/2018 18:56, Andy Shevchenko wrote: > On Mon, May 14, 2018 at 1:57 PM, Richard Genoud > wrote: >> On 11/05/2018 12:38, Radu Pirea wrote: >>> This patch modifies the place where resources and device tree properties >>> are searched. > >> I think it may be simpler with something like: > >> + int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + >> 1; > > Isn't resource_size() macro for this very purpose? Indeed. + int size = resource_size(mfd_pdev->resource); would be even simpler ! > >>> + int size = to_platform_device(pdev->dev.parent)->resource[0].end - >>> + to_platform_device(pdev->dev.parent)->resource[0].start + 1; >>> >> ditto > > Ditto. > Thanks ! Richard.
Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
Hi,
On 11/05/2018 12:38, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
>
> Signed-off-by: Radu Pirea
> ---
> drivers/tty/serial/Kconfig| 1 +
> drivers/tty/serial/atmel_serial.c | 29 +++--
> 2 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
> depends on ARCH_AT91 || COMPILE_TEST
> select SERIAL_CORE
> select SERIAL_MCTRL_GPIO if GPIOLIB
> + select MFD_AT91_USART
> help
> This enables the driver for the on-chip UARTs of the Atmel
> AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..6b4494352853 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>
> #if defined(CONFIG_OF)
> static const struct of_device_id atmel_serial_dt_ids[] = {
> - { .compatible = "atmel,at91rm9200-usart" },
> - { .compatible = "atmel,at91sam9260-usart" },
> + { .compatible = "atmel,at91rm9200-usart-serial" },
> + { .compatible = "atmel,at91sam9260-usart-serial" },
> { /* sentinel */ }
> };
> #endif
> @@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
> static void atmel_init_property(struct atmel_uart_port *atmel_port,
> struct platform_device *pdev)
> {
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *np = pdev->dev.parent->of_node;
>
> /* DMA/PDC usage specification */
> if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -2223,7 +2223,8 @@ static const char *atmel_type(struct uart_port *port)
> static void atmel_release_port(struct uart_port *port)
> {
> struct platform_device *pdev = to_platform_device(port->dev);
> - int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> + int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> + to_platform_device(pdev->dev.parent)->resource[0].start + 1;
I think it may be simpler with something like:
+ struct platform_device *mfd_pdev =
to_platform_device(port->dev->parent);
+ int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;
>
> release_mem_region(port->mapbase, size);
>
> @@ -2239,7 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
> static int atmel_request_port(struct uart_port *port)
> {
> struct platform_device *pdev = to_platform_device(port->dev);
> - int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> + int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> + to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>
ditto
> if (!request_mem_region(port->mapbase, size, "atmel_serial"))
> return -EBUSY;
> @@ -2345,23 +2347,23 @@ static int atmel_init_port(struct atmel_uart_port
> *atmel_port,
Here, we could also add:
+ struct device *mfd_dev = pdev->dev.parent;
+ struct platform_device *mfd_pdev = to_platform_device(mfd_dev);
> atmel_init_property(atmel_port, pdev);
> atmel_set_ops(port);
>
> - uart_get_rs485_mode(&pdev->dev, &port->rs485);
> + uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
...and use them here
>
> port->iotype= UPIO_MEM;
> port->flags = UPF_BOOT_AUTOCONF | UPF_IOREMAP;
> port->ops = &atmel_pops;
> port->fifosize = 1;
> port->dev = &pdev->dev;
> - port->mapbase = pdev->resource[0].start;
> - port->irq = pdev->resource[1].start;
> + port->mapbase =
> to_platform_device(pdev->dev.parent)->resource[0].start;
> + port->irq =
> to_platform_device(pdev->dev.parent)->resource[1].start;
and here
I think it would be easier to read.
> port->rs485_config = atmel_config_rs485;
> - port->membase = NULL;
> + port->membase = NULL;
>
> memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
>
> /* for console, the clock could already be configured */
> if (!atmel_port->clk) {
> - atmel_port->clk = clk_get(&pdev->dev, "usart");
> + atmel_port->clk = clk_get(pdev->dev.parent, "usart");
and here
> if (IS_ERR(atmel_port->clk)) {
> ret = PTR_ERR(atmel_port->clk);
> atmel_port->clk = NULL;
> @@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct
> atmel_uart_port *atmel_port,
> atmel_port->rts_low = 0;
> atmel_port->rts_high = 0;
>
> - if (of_property_read_u32(pdev->dev.of_nod
[PATCH v2] clk: mvebu: armada-38x: add support for missing clocks
Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
or 1333MHz (industrial).
They have also some dip-switches to select a different clock (666, 800,
1066, 1200).
The funny thing is that the recovery button is on the MPP34 fq selector.
So, when booting an industrial board with this button down, the frequency
666MHz is selected (and the kernel didn't boot).
This patch add all the missing clocks.
The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
Fixes: 0e85aeced4d6 ("clk: mvebu: add clock support for Armada 380/385")
Cc: # 3.16.x: 9593f4f56cf5: clk: mvebu: armada-38x:
add support for 1866MHz variants
Cc: # 3.16.x
Signed-off-by: Richard Genoud
Acked-by: Gregory CLEMENT
---
drivers/clk/mvebu/armada-38x.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
index 394aa6f03f01..9ff4ea63932d 100644
--- a/drivers/clk/mvebu/armada-38x.c
+++ b/drivers/clk/mvebu/armada-38x.c
@@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem
*sar)
}
static const u32 armada_38x_cpu_frequencies[] __initconst = {
- 0, 0, 0, 0,
- 1066 * 1000 * 1000, 0, 0, 0,
+ 666 * 1000 * 1000, 0, 800 * 1000 * 1000, 0,
+ 1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
1332 * 1000 * 1000, 0, 0, 0,
1600 * 1000 * 1000, 0, 0, 0,
- 1866 * 1000 * 1000,
+ 1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
};
static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
@@ -76,11 +76,11 @@ static const struct coreclk_ratio
armada_38x_coreclk_ratios[] __initconst = {
};
static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
- {0, 1}, {0, 1}, {0, 1}, {0, 1},
- {1, 2}, {0, 1}, {0, 1}, {0, 1},
- {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {1, 2}, {0, 1},
+ {1, 2}, {0, 1}, {1, 2}, {0, 1},
{1, 2}, {0, 1}, {0, 1}, {0, 1},
{1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {1, 2},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
@@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst
= {
{1, 2}, {0, 1}, {0, 1}, {0, 1},
{1, 2}, {0, 1}, {0, 1}, {0, 1},
{1, 2}, {0, 1}, {0, 1}, {0, 1},
- {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {7, 15},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
Re: [PATCH] clk: mvebu: armada-38x: add support for missing clocks
On 08/03/2018 14:23, Gregory CLEMENT wrote:
> Hi,
>
> On jeu., mars 08 2018, Gregory CLEMENT wrote:
>
>> Hi Richard,
>>
>> On jeu., mars 08 2018, Richard Genoud wrote:
>>
>>> Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
>>> or 1333MHz (industrial).
>>>
>>> They have also some dip-switches to select a different clock (666, 800,
>>> 1066, 1200).
>>
>> The patch looks goo and it will also be usefull for any other board
>> using these frequencies, thanks for this. I have only one small comment,
>> see below.
>
>
> I forgot to mention that you can add my
>
> Acked-by: Gregory CLEMENT
>
> Thanks,
>
> Gregory
>
>>
>>
>>>
>>> The funny thing is that the recovery button is on the MPP34 fq selector.
>>> So, when booting an industrial board with this button down, the frequency
>>> 666MHz is selected (and the kernel didn't boot).
>>>
>>> This patch add all the missing clocks.
>>>
>>> The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
>>>
>>> Signed-off-by: Richard Genoud
>>> ---
>>> drivers/clk/mvebu/armada-38x.c | 14 +++---
>>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
>>> index 394aa6f03f01..9ff4ea63932d 100644
>>> --- a/drivers/clk/mvebu/armada-38x.c
>>> +++ b/drivers/clk/mvebu/armada-38x.c
>>> @@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem
>>> *sar)
>>> }
>>>
>>> static const u32 armada_38x_cpu_frequencies[] __initconst = {
>>> - 0, 0, 0, 0,
>>> - 1066 * 1000 * 1000, 0, 0, 0,
>>> + 666 * 1000 * 1000, 0, 800 * 1000 * 1000, 0,
>>> + 1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
>>> 1332 * 1000 * 1000, 0, 0, 0,
>>> 1600 * 1000 * 1000, 0, 0, 0,
>>> - 1866 * 1000 * 1000,
>>> + 1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
>>
>> Maybe you could add a comment here to say that the 2GHz mode didn't have
>> been tested.
Well, if I add a comment there saying "2GHz mode hasn't been tested",
I'm afraid it will stay there forever, even after being tested.
(if it's working, nobody will look at it, and if it's not, it's in the commit
message anyway).
I was also thinking that this could go in -stable since it fixes a hang.
In this case, commit 9593f4f56cf5 ("clk: mvebu: armada-38x: add support for
1866MHz variants")
should also go with it.
Thanks !
>>
>> Thanks,
>> Gregory
>>
>>
>>> };
>>>
>>> static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
>>> @@ -76,11 +76,11 @@ static const struct coreclk_ratio
>>> armada_38x_coreclk_ratios[] __initconst = {
>>> };
>>>
>>> static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
>>> - {0, 1}, {0, 1}, {0, 1}, {0, 1},
>>> - {1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> - {1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> + {1, 2}, {0, 1}, {1, 2}, {0, 1},
>>> + {1, 2}, {0, 1}, {1, 2}, {0, 1},
>>> {1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> {1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> + {1, 2}, {0, 1}, {0, 1}, {1, 2},
>>> {0, 1}, {0, 1}, {0, 1}, {0, 1},
>>> {0, 1}, {0, 1}, {0, 1}, {0, 1},
>>> {0, 1}, {0, 1}, {0, 1}, {0, 1},
>>> @@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2]
>>> __initconst = {
>>> {1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> {1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> {1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> - {1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> + {1, 2}, {0, 1}, {0, 1}, {7, 15},
>>> {0, 1}, {0, 1}, {0, 1}, {0, 1},
>>> {0, 1}, {0, 1}, {0, 1}, {0, 1},
>>> {0, 1}, {0, 1}, {0, 1}, {0, 1},
>>
>> --
>> Gregory Clement, Bootlin (formerly Free Electrons)
>> Embedded Linux and Kernel engineering
>> http://bootlin.com
>
[PATCH] clk: mvebu: armada-38x: add support for missing clocks
Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
or 1333MHz (industrial).
They have also some dip-switches to select a different clock (666, 800,
1066, 1200).
The funny thing is that the recovery button is on the MPP34 fq selector.
So, when booting an industrial board with this button down, the frequency
666MHz is selected (and the kernel didn't boot).
This patch add all the missing clocks.
The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
Signed-off-by: Richard Genoud
---
drivers/clk/mvebu/armada-38x.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
index 394aa6f03f01..9ff4ea63932d 100644
--- a/drivers/clk/mvebu/armada-38x.c
+++ b/drivers/clk/mvebu/armada-38x.c
@@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem
*sar)
}
static const u32 armada_38x_cpu_frequencies[] __initconst = {
- 0, 0, 0, 0,
- 1066 * 1000 * 1000, 0, 0, 0,
+ 666 * 1000 * 1000, 0, 800 * 1000 * 1000, 0,
+ 1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
1332 * 1000 * 1000, 0, 0, 0,
1600 * 1000 * 1000, 0, 0, 0,
- 1866 * 1000 * 1000,
+ 1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
};
static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
@@ -76,11 +76,11 @@ static const struct coreclk_ratio
armada_38x_coreclk_ratios[] __initconst = {
};
static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
- {0, 1}, {0, 1}, {0, 1}, {0, 1},
- {1, 2}, {0, 1}, {0, 1}, {0, 1},
- {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {1, 2}, {0, 1},
+ {1, 2}, {0, 1}, {1, 2}, {0, 1},
{1, 2}, {0, 1}, {0, 1}, {0, 1},
{1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {1, 2},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
@@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst
= {
{1, 2}, {0, 1}, {0, 1}, {0, 1},
{1, 2}, {0, 1}, {0, 1}, {0, 1},
{1, 2}, {0, 1}, {0, 1}, {0, 1},
- {1, 2}, {0, 1}, {0, 1}, {0, 1},
+ {1, 2}, {0, 1}, {0, 1}, {7, 15},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
{0, 1}, {0, 1}, {0, 1}, {0, 1},
Re: Soft lockup in rt2x00usb_work_rxdone()
Le mercredi 08 novembre 2017 à 11:37 +0100, Stanislaw Gruszka a écrit : > On Tue, Nov 07, 2017 at 12:01:23PM +0100, Richard Genoud wrote: > > Le mardi 07 novembre 2017 à 11:13 +0100, Stanislaw Gruszka a > > écrit : > > > On Tue, Nov 07, 2017 at 11:06:39AM +0100, Richard Genoud wrote: > > > > > 3 short articles how to configure and use ftrace are here: > > > > > https://lwn.net/Articles/365835/ > > > > > https://lwn.net/Articles/366796/ > > > > > https://lwn.net/Articles/370423/ > > > > > > > > > > > > > I tried with ftrace, but I don't think there's a way to dump > > > > the > > > > trace > > > > when there's a soft lock-up > > > > (I can't do anything after the unbind, even the heartbeat led > > > > stopped blinking). > > > > I saw the /proc/sys/kernel/ftrace_dump_on_oops file, but > > > > there's no > > > > /proc/sys/kernel/ftrace_dump_on_soft_lock-up file :) > > > > > > You should configure function trace with rt2x* functions. After > > > that > > > start tracing, unbind the device, then stop tracing and provide > > > trace > > > output. > > > > Ok, I found a way to display the trace (after the unbind, the board > > is > > frozen and I can't type anything). > > Adding > > CONFIG_SOFTLOCKUP_DETECTOR=y > > CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC=y > > along with echo 1 > /proc/sys/kernel/ftrace_dump_on_oops does the > > trick > > No, that not I wanted you to do. Please remove those options and just > do > below on tracing directory. > > echo 0 > tracing_on > cat trace > /dev/null > echo "function_graph" > current_tracer > echo "rt2*" > set_ftrace_filter > echo 1 > tracing_on > echo 1-2.2 > /sys/bus/usb/drivers/usb/unbind > echo 0 > tracing_on > cat trace > ~/trace.txt Well, there's clearly a misunderstanding here : After the command "echo 1-2.2 > /sys/bus/usb/drivers/usb/unbind" I can't type *anything* The only thing I can do is plug off the board. This command never returns, so I can't stop the tracing... Or I missed something ? (maybe if there was more than one CPU on the board, I could do something, but that's not the case) > > and provide trace.txt to me (can be in private email if big). > > Thanks > Stanislaw Thanks, Richard.
Re: Soft lockup in rt2x00usb_work_rxdone()
Le mardi 07 novembre 2017 à 11:13 +0100, Stanislaw Gruszka a écrit :
> On Tue, Nov 07, 2017 at 11:06:39AM +0100, Richard Genoud wrote:
> > > 3 short articles how to configure and use ftrace are here:
> > > https://lwn.net/Articles/365835/
> > > https://lwn.net/Articles/366796/
> > > https://lwn.net/Articles/370423/
> > >
> >
> > I tried with ftrace, but I don't think there's a way to dump the
> > trace
> > when there's a soft lock-up
> > (I can't do anything after the unbind, even the heartbeat led
> > stopped blinking).
> > I saw the /proc/sys/kernel/ftrace_dump_on_oops file, but there's no
> > /proc/sys/kernel/ftrace_dump_on_soft_lock-up file :)
>
> You should configure function trace with rt2x* functions. After that
> start tracing, unbind the device, then stop tracing and provide trace
> output.
Here is another trace, with rt2* as function filter.
(sorry for the noise)
Dumping ftrace buffer:
-
CPU:0 [LOST 3606923 EVENTS]
0) 0.000 us| } /* rt2x00usb_clear_entry */
0) 0.000 us|} /* rt2x00lib_rxdone */
0) 0.000 us|rt2x00queue_get_entry();
0) |rt2x00lib_rxdone() {
0) 0.000 us| rt2x00queue_index_inc();
0) | rt2x00usb_clear_entry() {
0) |rt2x00usb_kick_rx_entry() {
0) | rt2x00lib_dmastart() {
0) 0.000 us|rt2x00queue_index_inc();
0) 0.000 us| }
0) | rt2x00lib_dmadone() {
0) 0.000 us|rt2x00queue_index_inc();
0) 0.000 us| }
0) 0.000 us|}
0) 0.000 us| }
0) 0.000 us|}
0) 0.000 us|rt2x00queue_get_entry();
0) |rt2x00lib_rxdone() {
0) 0.000 us| rt2x00queue_index_inc();
0) | rt2x00usb_clear_entry() {
0) |rt2x00usb_kick_rx_entry() {
0) | rt2x00lib_dmastart() {
0) 0.000 us|rt2x00queue_index_inc();
0) 0.000 us| }
0) | rt2x00lib_dmadone() {
0) 0.000 us|rt2x00queue_index_inc();
0) 0.000 us| }
0) 0.000 us|}
0) 0.000 us| }
0) 0.000 us|}
0) 0.000 us|rt2x00queue_get_entry();
0) |rt2x00lib_rxdone() {
0) 0.000 us| rt2x00queue_index_inc();
0) | rt2x00usb_clear_entry() {
0) |rt2x00usb_kick_rx_entry() {
0) | rt2x00lib_dmastart() {
0) 0.000 us|rt2x00queue_index_inc();
0) 0.000 us| }
0) | rt2x00lib_dmadone() {
0) 0.000 us|rt2x00queue_index_inc();
0) 0.000 us| }
0) 0.000 us|}
0) 0.000 us| }
0) 0.000 us|}
>
> Thanks
> Stanislaw
Re: Soft lockup in rt2x00usb_work_rxdone()
Le mardi 07 novembre 2017 à 11:13 +0100, Stanislaw Gruszka a écrit : > On Tue, Nov 07, 2017 at 11:06:39AM +0100, Richard Genoud wrote: > > > 3 short articles how to configure and use ftrace are here: > > > https://lwn.net/Articles/365835/ > > > https://lwn.net/Articles/366796/ > > > https://lwn.net/Articles/370423/ > > > > > > > I tried with ftrace, but I don't think there's a way to dump the > > trace > > when there's a soft lock-up > > (I can't do anything after the unbind, even the heartbeat led > > stopped blinking). > > I saw the /proc/sys/kernel/ftrace_dump_on_oops file, but there's no > > /proc/sys/kernel/ftrace_dump_on_soft_lock-up file :) > > You should configure function trace with rt2x* functions. After that > start tracing, unbind the device, then stop tracing and provide trace > output. Ok, I found a way to display the trace (after the unbind, the board is frozen and I can't type anything). Adding CONFIG_SOFTLOCKUP_DETECTOR=y CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC=y along with echo 1 > /proc/sys/kernel/ftrace_dump_on_oops does the trick (trace is after the stack dump) # cd /sys/kernel/debug/tracing/ # echo 1 > /proc/sys/kernel/ftrace_dump_on_oops # # echo rt2x00usb* > set_ftrace_filter # echo 0 > tracing_on # echo function > current_tracer # echo 1 > tracing_on # echo 1-2.2 > /sys/bus/usb/drivers/usb/unbind [board frozen] watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [kworker/u2:3:188] CPU: 0 PID: 188 Comm: kworker/u2:3 Not tainted 4.14.0-rc8-00040-g53fb1fe423ba #13 Hardware name: Atmel AT91SAM9 Workqueue: phy0 rt2x00usb_work_rxdone task: c7b34400 task.stack: c7b4e000 PC is at rb_commit+0x1a8/0x2e4 LR is at ring_buffer_unlock_commit+0x20/0xa4 pc : []lr : []psr: 8013 sp : c7b4fda8 ip : fp : c7b4fdc4 r10: c664ee34 r9 : c7b2ed18 r8 : 6013 r7 : 001c4851 r6 : c780a0e0 r5 : c7319340 r4 : 8a48 r3 : c7319340 r2 : c664e000 r1 : 0e38 r0 : 0e24 Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 0005317f Table: 2727 DAC: 0053 CPU: 0 PID: 188 Comm: kworker/u2:3 Not tainted 4.14.0-rc8-00040-g53fb1fe423ba #13 Hardware name: Atmel AT91SAM9 Workqueue: phy0 rt2x00usb_work_rxdone [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (dump_stack+0x20/0x28) [] (dump_stack) from [] (show_regs+0x1c/0x20) [] (show_regs) from [] (watchdog_timer_fn+0x148/0x1ac) [] (watchdog_timer_fn) from [] (hrtimer_run_queues+0x128/0x250) [] (hrtimer_run_queues) from [] (run_local_timers+0x18/0x68) [] (run_local_timers) from [] (update_process_times+0x38/0x6c) [] (update_process_times) from [] (tick_nohz_handler+0xc0/0x10c) [] (tick_nohz_handler) from [] (ch2_irq+0x30/0x38) [] (ch2_irq) from [] (__handle_irq_event_percpu+0x74/0x1dc) [] (__handle_irq_event_percpu) from [] (handle_irq_event_percpu+0x2c/0x68) [] (handle_irq_event_percpu) from [] (handle_irq_event+0x38/0x4c) [] (handle_irq_event) from [] (handle_fasteoi_irq+0xa0/0x114) [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x28/0x38) [] (generic_handle_irq) from [] (__handle_domain_irq+0x90/0xb8) [] (__handle_domain_irq) from [] (aic_handle+0xb0/0xb8) [] (aic_handle) from [] (__irq_svc+0x68/0x84) Exception stack(0xc7b4fd58 to 0xc7b4fda0) fd40: 0e24 0e38 fd60: c664e000 c7319340 8a48 c7319340 c780a0e0 001c4851 6013 c7b2ed18 fd80: c664ee34 c7b4fdc4 c7b4fda8 c006d724 c006c694 8013 [] (__irq_svc) from [] (rb_commit+0x1a8/0x2e4) [] (rb_commit) from [] (ring_buffer_unlock_commit+0x20/0xa4) [] (ring_buffer_unlock_commit) from [] (trace_function+0xe0/0xf0) [] (trace_function) from [] (function_trace_call+0xbc/0x11c) [] (function_trace_call) from [] (ftrace_graph_call+0x0/0xc) [] (ftrace_graph_call) from [] (rt2x00usb_kick_rx_entry+0x14/0x118) [] (rt2x00usb_kick_rx_entry) from [] (rt2x00usb_clear_entry+0x30/0x34) [] (rt2x00usb_clear_entry) from [] (rt2x00lib_rxdone+0x58c/0x5b8) [] (rt2x00lib_rxdone) from [] (rt2x00usb_work_rxdone+0x60/0x7c) [] (rt2x00usb_work_rxdone) from [] (process_one_work+0x1e4/0x3a0) [] (process_one_work) from [] (worker_thread+0x2c8/0x45c) [] (worker_thread) from [] (kthread+0x114/0x130) [] (kthread) from [] (ret_from_fork+0x14/0x2c) Kernel panic - not syncing: softlockup: hung tasks CPU: 0 PID: 188 Comm: kworker/u2:3 Tainted: G L 4.14.0-rc8-00040-g53fb1fe423ba #13 Hardware name: Atmel AT91SAM9 Workqueue: phy0 rt2x00usb_work_rxdone [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (dump_stack+0x20/0x28) [] (dump_stack) from [] (panic+0xc8/0x260) [] (panic) from [] (watchdog_timer_fn+0x180/0x1ac) [] (watchdog_timer_fn) from [] (hrtimer_run_queues+0x128/0x250) [] (hrtimer_run_queues) from [] (run_local_timers+0x18/0x68) [] (run_local_timers
Re: Soft lockup in rt2x00usb_work_rxdone()
2017-11-07 9:53 GMT+01:00 Stanislaw Gruszka : > Hi Hi ! > > On Mon, Nov 06, 2017 at 04:57:09PM +0100, Richard Genoud wrote: >> I get a soft lock-up while unbinding the USB driver on a TP-Link TL-WN727Nv3 >> (chipset 5370): >> >> # echo 1-2.2 > /sys/bus/usb/drivers/usb/unbind >> watchdog: BUG: soft lockup - CPU#0 stuck for 23s! [kworker/u2:3:308] > ... >> I can trigger this each time. > > I can not reproduce this on my system (I'm using 4.14.0-rc6, but I don't > think it's an issue). I think the problem may be caused by usb > host controler driver, which can be different on your system. > > Does ftrace work on your platform ? If so could you use ftrace > to provide rt2x00 functions trace when the probllem happen ? > > 3 short articles how to configure and use ftrace are here: > https://lwn.net/Articles/365835/ > https://lwn.net/Articles/366796/ > https://lwn.net/Articles/370423/ > I tried with ftrace, but I don't think there's a way to dump the trace when there's a soft lock-up (I can't do anything after the unbind, even the heartbeat led stopped blinking). I saw the /proc/sys/kernel/ftrace_dump_on_oops file, but there's no /proc/sys/kernel/ftrace_dump_on_soft_lock-up file :) Or I missed something in ftrace ? Thanks ! Richard. > Thanks > Stanislaw
Soft lockup in rt2x00usb_work_rxdone()
Hi, I get a soft lock-up while unbinding the USB driver on a TP-Link TL-WN727Nv3 (chipset 5370): # echo 1-2.2 > /sys/bus/usb/drivers/usb/unbind watchdog: BUG: soft lockup - CPU#0 stuck for 23s! [kworker/u2:3:308] CPU: 0 PID: 308 Comm: kworker/u2:3 Not tainted 4.14.0-rc8 #11 Hardware name: Atmel AT91SAM9 Workqueue: phy0 rt2x00usb_work_rxdone task: c7b91000 task.stack: c7bee000 PC is at rt2x00lib_rxdone+0x590/0x5b8 LR is at rt2x00lib_dmadone+0x54/0x58 pc : []lr : []psr: 8013 sp : c7befebc ip : 0052 fp : c7befee4 r10: r9 : c79b2b58 r8 : r7 : c7816d00 r6 : c780e200 r5 : c79c68dc r4 : c7134ce0 r3 : 0001 r2 : c70bc200 r1 : a55f r0 : Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none Control: 0005317f Table: 261b8000 DAC: 0053 CPU: 0 PID: 308 Comm: kworker/u2:3 Not tainted 4.14.0-rc8 #11 Hardware name: Atmel AT91SAM9 Workqueue: phy0 rt2x00usb_work_rxdone [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (dump_stack+0x20/0x28) [] (dump_stack) from [] (show_regs+0x1c/0x20) [] (show_regs) from [] (watchdog_timer_fn+0x148/0x1ac) [] (watchdog_timer_fn) from [] (hrtimer_run_queues+0x128/0x250) [] (hrtimer_run_queues) from [] (run_local_timers+0x18/0x68) [] (run_local_timers) from [] (update_process_times+0x38/0x6c) [] (update_process_times) from [] (tick_nohz_handler+0xc0/0x10c) [] (tick_nohz_handler) from [] (ch2_irq+0x30/0x38) [] (ch2_irq) from [] (__handle_irq_event_percpu+0x74/0x1dc) [] (__handle_irq_event_percpu) from [] (handle_irq_event_percpu+0x2c/0x68) [] (handle_irq_event_percpu) from [] (handle_irq_event+0x38/0x4c) [] (handle_irq_event) from [] (handle_fasteoi_irq+0xa0/0x114) [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x28/0x38) [] (generic_handle_irq) from [] (__handle_domain_irq+0x90/0xb8) [] (__handle_domain_irq) from [] (aic_handle+0xb0/0xb8) [] (aic_handle) from [] (__irq_svc+0x68/0x84) Exception stack(0xc7befe68 to 0xc7befeb0) fe60: a55f c70bc200 0001 c7134ce0 c79c68dc fe80: c780e200 c7816d00 c79b2b58 c7befee4 0052 c7befebc fea0: c0331fdc c0332978 8013 [] (__irq_svc) from [] (rt2x00lib_rxdone+0x590/0x5b8) [] (rt2x00lib_rxdone) from [] (rt2x00usb_work_rxdone+0x60/0x7c) [] (rt2x00usb_work_rxdone) from [] (process_one_work+0x1e4/0x3a0) [] (process_one_work) from [] (worker_thread+0x2c8/0x45c) [] (worker_thread) from [] (kthread+0x114/0x130) [] (kthread) from [] (ret_from_fork+0x14/0x2c) I can trigger this each time. NB: if the wifi is deactivated properly before the unbind, there's no problem : # ifconfig wlan0 down ; echo 1-2.2 > /sys/bus/usb/drivers/usb/unbind wlan0: deauthenticating from 06:18:d6:91:9e:29 by local choice (Reason: 3=DEAUTH_LEAVING) Full dmesg: ## Booting kernel from Legacy Image at 20007fc0 ... Image Name: Linux-4.14.0-rc8 Image Type: ARM Linux Kernel Image (uncompressed) Data Size:4522192 Bytes = 4.3 MiB Load Address: 20008000 Entry Point: 20008000 ## Flattened Device Tree blob at 2640 Booting using the fdt blob at 0x2640 XIP Kernel Image ... OK Loading Device Tree to 27df2000, end 27dfcbb3 ... OK Starting kernel ... Booting Linux on physical CPU 0x0 Linux version 4.14.0-rc8 (rgenoud@lnx-rg) (gcc version 4.7.3 (GCC)) #11 Mon Nov 6 16:22:49 CET 2017 CPU: ARM926EJ-S [41069265] revision 5 (ARMv5TEJ), cr=0005317f CPU: VIVT data cache, VIVT instruction cache OF: fdt: Machine model: Paratronic LNS Memory policy: Data cache writeback On node 0 totalpages: 32768 free_area_init_node: node 0, pgdat c08a4b78, node_mem_map c7efb000 Normal zone: 256 pages used for memmap Normal zone: 0 pages reserved Normal zone: 32768 pages, LIFO batch:7 random: fast init done pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768 pcpu-alloc: [0] 0 Built 1 zonelists, mobility grouping on. Total pages: 32512 Kernel command line: loglevel=8 ro root=ubi0:rootfs rootfstype=ubifs ubi.mtd=ubi lpj=995328 ubi.fm_autoconvert=1 panic=2 mtdparts=atmel_nand:256M(all),128k@0(dtb),10112k(kernel),251392k(ubi),512k(bbt)ro atmel-nand-controller.use_dma=0 spidev.bufsiz=53200 video=320x240-32 PID hash table entries: 512 (order: -1, 2048 bytes) Dentry cache hash table entries: 16384 (order: 4, 65536 bytes) Inode-cache hash table entries: 8192 (order: 3, 32768 bytes) Memory: 120624K/131072K available (5847K kernel code, 475K rwdata, 1504K rodata, 480K init, 245K bss, 10448K reserved, 0K cma-reserved) Virtual kernel memory layout: vector : 0x - 0x1000 ( 4 kB) fixmap : 0xffc0 - 0xfff0 (3072 kB) vmalloc : 0xc880 - 0xff80 ( 880 MB) lowmem : 0xc000 - 0xc800 ( 128 MB) .text : 0xc0008000 - 0xc05be200 (5849 kB) .init : 0xc07be000 - 0xc0836000 ( 480 kB) .data : 0xc0836000 - 0xc08accd0 ( 476 kB) .bss : 0xc08b20e8 - 0xc08ef77c ( 246 kB) ftrace: allocating 23621 entries in 70 pages NR_IRQS: 16, nr_
[PATCH] tty/serial: atmel: Convert timers to use timer_setup()
Le mardi 24 octobre 2017 à 03:00 -0700, Kees Cook a écrit :
> In preparation for unconditionally passing the struct timer_list
> pointer to
> all timer callbacks, switch to using the new timer_setup() and
> from_timer()
> to pass the timer pointer explicitly.
>
> Cc: Richard Genoud
> Cc: Greg Kroah-Hartman
> Cc: Jiri Slaby
> Cc: linux-ser...@vger.kernel.org
> Signed-off-by: Kees Cook
Acked-by: Richard Genoud
Thanks !
> ---
> drivers/tty/serial/atmel_serial.c | 11 +--
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 82d9c8eae04f..68d8685e5a50 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -1185,10 +1185,11 @@ static int atmel_prepare_rx_dma(struct
> uart_port *port)
> return -EINVAL;
> }
>
> -static void atmel_uart_timer_callback(unsigned long data)
> +static void atmel_uart_timer_callback(struct timer_list *t)
> {
> - struct uart_port *port = (void *)data;
> - struct atmel_uart_port *atmel_port =
> to_atmel_uart_port(port);
> + struct atmel_uart_port *atmel_port = from_timer(atmel_port,
> t,
> + uart_timer);
> + struct uart_port *port = &atmel_port->uart;
>
> if (!atomic_read(&atmel_port->tasklet_shutdown)) {
> tasklet_schedule(&atmel_port->tasklet_rx);
> @@ -1852,9 +1853,7 @@ static int atmel_startup(struct uart_port
> *port)
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN |
> ATMEL_US_RXEN);
> atmel_port->tx_stopped = false;
>
> - setup_timer(&atmel_port->uart_timer,
> - atmel_uart_timer_callback,
> - (unsigned long)port);
> + timer_setup(&atmel_port->uart_timer,
> atmel_uart_timer_callback, 0);
>
> if (atmel_use_pdc_rx(port)) {
> /* set UART timeout */
> --
> 2.7.4
>
>
[PATCH] mtd: nand: atmel: fix buffer overflow in atmel_pmecc_user
When calculating the size needed by struct atmel_pmecc_user *user,
the dmu and delta buffer sizes were forgotten.
This lead to a memory corruption (especially with a large ecc_strength).
Link: http://lkml.kernel.org/r/1506503157.3016.5.ca...@gmail.com
Fixes: f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")
Cc: Nicolas Ferre
Cc: sta...@vger.kernel.org
Reported-by: Richard Genoud
Pointed-at-by: Boris Brezillon
Signed-off-by: Richard Genoud
---
drivers/mtd/nand/atmel/pmecc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/atmel/pmecc.c b/drivers/mtd/nand/atmel/pmecc.c
index 146af8218314..8268636675ef 100644
--- a/drivers/mtd/nand/atmel/pmecc.c
+++ b/drivers/mtd/nand/atmel/pmecc.c
@@ -363,7 +363,7 @@ atmel_pmecc_create_user(struct atmel_pmecc *pmecc,
size += (req->ecc.strength + 1) * sizeof(u16);
/* Reserve space for mu, dmu and delta. */
size = ALIGN(size, sizeof(s32));
- size += (req->ecc.strength + 1) * sizeof(s32);
+ size += (req->ecc.strength + 1) * sizeof(s32) * 3;
user = kzalloc(size, GFP_KERNEL);
if (!user)
Re: atmel_nand: kernel panic when ecc_strength==4
2017-09-27 12:15 GMT+02:00 Richard Genoud :
> 2017-09-27 12:04 GMT+02:00 Boris Brezillon
> :
>> On Wed, 27 Sep 2017 11:05:57 +0200
>> Richard Genoud wrote:
>>
>>> Hi Boris, Nicolas !
>>>
>>> Since commit f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand
>>> driver")
>>> strange things happen when nand-ecc-strength = <4>; (previously
>>> atmel,pmecc-cap).
>>>
>>> I first saw that a NULL pointer dereference happened when "udevadm trigger"
>>> was launched.
>>> With strace, I nailed it down to :
>>>
>>> sh-4.3# echo change > /sys/devices/virtual/bdi/mtd-1/uevent
>>> [ 86.696275] Unable to handle kernel NULL pointer dereference at virtual
>>> address 0001
>>> [ 86.704285] pgd = c717c000
>>> [ 86.707072] [0001] *pgd=c06d9a70, *pte=,
>>> *ppte=
>>> [ 86.713979] Internal error: Oops: 17 [#3] ARM
>>> [ 86.718306] CPU: 0 PID: 1 Comm: sh Tainted: G D W
>>> 4.11.0-rc1-00056-gf88fc122cc34-dirty #75
>>> [ 86.727443] Hardware name: Atmel AT91SAM9
>>> [ 86.731424] task: c7880b60 task.stack: c7884000
>>> [ 86.735926] PC is at strlen+0x14/0x2c
>>> [ 86.739556] LR is at kobject_get_path+0x34/0xac
>>> [ 86.744046] pc : []lr : []psr: 2013
>>> [ 86.744046] sp : c7885dc0 ip : c7885dd0 fp : c7885dcc
>>> [ 86.755439] r10: 0002 r9 : r8 : c7885f78
>>> [ 86.760627] r7 : 014000c0 r6 : c7ab2308 r5 : 0001 r4 : c7ab2308
>>> [ 86.767106] r3 : 0001 r2 : 0001 r1 : 014000c0 r0 : 0001
>>> [ 86.773588] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment
>>> none
>>> [ 86.780672] Control: 0005317f Table: 2717c000 DAC: 0051
>>> [ 86.786372] Process sh (pid: 1, stack limit = 0xc7884190)
>>> [ 86.791730] Stack: (0xc7885dc0 to 0xc7886000)
>>> [ 86.796075] 5dc0: c7885df4 c7885dd0 c0235020 c023bc04 c0728bf8 c79f1000
>>> c7ab2308 c78c2b00
>>> [ 86.804195] 5de0: c04f4610 c7885f78 c7885e44 c7885df8 c0236244 c0234ffc
>>> c00a53b4 0074
>>> [ 86.812315] 5e00: 00107000 c7885ea8 c7885e64 c05d604b c717b420 c05b4aa8
>>> 081f 0007
>>> [ 86.820438] 5e20: c7ab2300 c7199ea0 c79baae0 c7885f78 c7199eb0 0007
>>> c7885e54 c7885e48
>>> [ 86.828559] 5e40: c0236640 c0236188 c7885e74 c7885e58 c02a5834 c023663c
>>> c7885e9c 0002
>>> [ 86.836681] 5e60: c7bf1f50 c79baae0 c7885e84 c7885e78 c02a37b8 c02a5800
>>> c7885e9c c7885e88
>>> [ 86.844801] 5e80: c0128fc8 c02a37a0 c7885ed4 c7885ea0
>>> c01281e4 c0128f8c
>>> [ 86.852922] 5ea0: c7880b60 c01280b8 00106cf8 c7215c20
>>> c7885f78 0007
>>> [ 86.861045] 5ec0: c7884000 00106cf8 c7885f44 c7885ed8 c00caec0 c01280c8
>>> 081f 00107d00
>>> [ 86.869167] 5ee0: c06d0f7c c7885fb0 00053177 1180 0178 c7885fac
>>> c7885f04 c00091e4
>>> [ 86.877288] 5f00: c001128c c000e088 0158 c00cb114 12bc
>>> bec504d0 b6e8bbec
>>> [ 86.885409] 5f20: c7215c20 c7215c20 0007 00106cf8 c7885f78
>>> c7885f74 c7885f48
>>> [ 86.893531] 5f40: c00cb160 c00cae94 c00e6e04 c00e6568
>>> c7215c20 c7215c20
>>> [ 86.901652] 5f60: 0007 00106cf8 c7885fa4 c7885f78 c00cb2dc c00cb0b0
>>>
>>> [ 86.909773] 5f80: 0007 00106cf8 b6e8dd50 0004 c000a544
>>> c7885fa8
>>> [ 86.917895] 5fa0: c000a3a0 c00cb2a0 0007 00106cf8 0001 00106cf8
>>> 0007
>>> [ 86.926015] 5fc0: 0007 00106cf8 b6e8dd50 0004 0007 0004
>>> 000e9124
>>> [ 86.934139] 5fe0: bec50a3c b6db63d0 b6e107ac 6010 0001
>>>
>>> [ 86.942277] [] (strlen) from []
>>> (kobject_get_path+0x34/0xac)
>>> [ 86.949620] [] (kobject_get_path) from []
>>> (kobject_uevent_env+0xcc/0x4b4)
>>> [ 86.958083] [] (kobject_uevent_env) from []
>>> (kobject_uevent+0x14/0x18)
>>> [ 86.966287] [] (kobject_uevent) from []
>>> (uevent_store+0x44/0x64)
>>> [ 86.973987] [] (uevent_store) from []
>>> (dev_attr_store+0x28/0x34)
>>> [ 86.981672] [] (dev_attr_store) from []
>>> (sysfs_kf_write+0x4c/0x58)
>>> [ 86.989525] [] (sysfs_kf_write) from []
>>> (kernfs_fop_write+0x12c/0x1c4)
>>&
Re: atmel_nand: kernel panic when ecc_strength==4
2017-09-27 12:04 GMT+02:00 Boris Brezillon :
> On Wed, 27 Sep 2017 11:05:57 +0200
> Richard Genoud wrote:
>
>> Hi Boris, Nicolas !
>>
>> Since commit f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")
>> strange things happen when nand-ecc-strength = <4>; (previously
>> atmel,pmecc-cap).
>>
>> I first saw that a NULL pointer dereference happened when "udevadm trigger"
>> was launched.
>> With strace, I nailed it down to :
>>
>> sh-4.3# echo change > /sys/devices/virtual/bdi/mtd-1/uevent
>> [ 86.696275] Unable to handle kernel NULL pointer dereference at virtual
>> address 0001
>> [ 86.704285] pgd = c717c000
>> [ 86.707072] [0001] *pgd=c06d9a70, *pte=,
>> *ppte=
>> [ 86.713979] Internal error: Oops: 17 [#3] ARM
>> [ 86.718306] CPU: 0 PID: 1 Comm: sh Tainted: G D W
>> 4.11.0-rc1-00056-gf88fc122cc34-dirty #75
>> [ 86.727443] Hardware name: Atmel AT91SAM9
>> [ 86.731424] task: c7880b60 task.stack: c7884000
>> [ 86.735926] PC is at strlen+0x14/0x2c
>> [ 86.739556] LR is at kobject_get_path+0x34/0xac
>> [ 86.744046] pc : []lr : []psr: 2013
>> [ 86.744046] sp : c7885dc0 ip : c7885dd0 fp : c7885dcc
>> [ 86.755439] r10: 0002 r9 : r8 : c7885f78
>> [ 86.760627] r7 : 014000c0 r6 : c7ab2308 r5 : 0001 r4 : c7ab2308
>> [ 86.767106] r3 : 0001 r2 : 0001 r1 : 014000c0 r0 : 0001
>> [ 86.773588] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment
>> none
>> [ 86.780672] Control: 0005317f Table: 2717c000 DAC: 0051
>> [ 86.786372] Process sh (pid: 1, stack limit = 0xc7884190)
>> [ 86.791730] Stack: (0xc7885dc0 to 0xc7886000)
>> [ 86.796075] 5dc0: c7885df4 c7885dd0 c0235020 c023bc04 c0728bf8 c79f1000
>> c7ab2308 c78c2b00
>> [ 86.804195] 5de0: c04f4610 c7885f78 c7885e44 c7885df8 c0236244 c0234ffc
>> c00a53b4 0074
>> [ 86.812315] 5e00: 00107000 c7885ea8 c7885e64 c05d604b c717b420 c05b4aa8
>> 081f 0007
>> [ 86.820438] 5e20: c7ab2300 c7199ea0 c79baae0 c7885f78 c7199eb0 0007
>> c7885e54 c7885e48
>> [ 86.828559] 5e40: c0236640 c0236188 c7885e74 c7885e58 c02a5834 c023663c
>> c7885e9c 0002
>> [ 86.836681] 5e60: c7bf1f50 c79baae0 c7885e84 c7885e78 c02a37b8 c02a5800
>> c7885e9c c7885e88
>> [ 86.844801] 5e80: c0128fc8 c02a37a0 c7885ed4 c7885ea0
>> c01281e4 c0128f8c
>> [ 86.852922] 5ea0: c7880b60 c01280b8 00106cf8 c7215c20
>> c7885f78 0007
>> [ 86.861045] 5ec0: c7884000 00106cf8 c7885f44 c7885ed8 c00caec0 c01280c8
>> 081f 00107d00
>> [ 86.869167] 5ee0: c06d0f7c c7885fb0 00053177 1180 0178 c7885fac
>> c7885f04 c00091e4
>> [ 86.877288] 5f00: c001128c c000e088 0158 c00cb114 12bc
>> bec504d0 b6e8bbec
>> [ 86.885409] 5f20: c7215c20 c7215c20 0007 00106cf8 c7885f78
>> c7885f74 c7885f48
>> [ 86.893531] 5f40: c00cb160 c00cae94 c00e6e04 c00e6568
>> c7215c20 c7215c20
>> [ 86.901652] 5f60: 0007 00106cf8 c7885fa4 c7885f78 c00cb2dc c00cb0b0
>>
>> [ 86.909773] 5f80: 0007 00106cf8 b6e8dd50 0004 c000a544
>> c7885fa8
>> [ 86.917895] 5fa0: c000a3a0 c00cb2a0 0007 00106cf8 0001 00106cf8
>> 0007
>> [ 86.926015] 5fc0: 0007 00106cf8 b6e8dd50 0004 0007 0004
>> 000e9124
>> [ 86.934139] 5fe0: bec50a3c b6db63d0 b6e107ac 6010 0001
>>
>> [ 86.942277] [] (strlen) from []
>> (kobject_get_path+0x34/0xac)
>> [ 86.949620] [] (kobject_get_path) from []
>> (kobject_uevent_env+0xcc/0x4b4)
>> [ 86.958083] [] (kobject_uevent_env) from []
>> (kobject_uevent+0x14/0x18)
>> [ 86.966287] [] (kobject_uevent) from []
>> (uevent_store+0x44/0x64)
>> [ 86.973987] [] (uevent_store) from []
>> (dev_attr_store+0x28/0x34)
>> [ 86.981672] [] (dev_attr_store) from []
>> (sysfs_kf_write+0x4c/0x58)
>> [ 86.989525] [] (sysfs_kf_write) from []
>> (kernfs_fop_write+0x12c/0x1c4)
>> [ 86.997737] [] (kernfs_fop_write) from []
>> (__vfs_write+0x3c/0x11c)
>> [ 87.005596] [] (__vfs_write) from []
>> (vfs_write+0xc0/0x164)
>> [ 87.012855] [] (vfs_write) from []
>> (SyS_write+0x4c/0x8c)
>> [ 87.019854] [] (SyS_write) from []
>> (ret_fast_syscall+0x0/0x38)
>> [ 87.027364] Code: e92dd800 e24cb004 e1a03000 e1a02003 (e5d21000)
>>
atmel_nand: kernel panic when ecc_strength==4
Hi Boris, Nicolas !
Since commit f88fc122cc34 ("mtd: nand: Cleanup/rework the atmel_nand driver")
strange things happen when nand-ecc-strength = <4>; (previously
atmel,pmecc-cap).
I first saw that a NULL pointer dereference happened when "udevadm trigger" was
launched.
With strace, I nailed it down to :
sh-4.3# echo change > /sys/devices/virtual/bdi/mtd-1/uevent
[ 86.696275] Unable to handle kernel NULL pointer dereference at virtual
address 0001
[ 86.704285] pgd = c717c000
[ 86.707072] [0001] *pgd=c06d9a70, *pte=, *ppte=
[ 86.713979] Internal error: Oops: 17 [#3] ARM
[ 86.718306] CPU: 0 PID: 1 Comm: sh Tainted: G D W
4.11.0-rc1-00056-gf88fc122cc34-dirty #75
[ 86.727443] Hardware name: Atmel AT91SAM9
[ 86.731424] task: c7880b60 task.stack: c7884000
[ 86.735926] PC is at strlen+0x14/0x2c
[ 86.739556] LR is at kobject_get_path+0x34/0xac
[ 86.744046] pc : []lr : []psr: 2013
[ 86.744046] sp : c7885dc0 ip : c7885dd0 fp : c7885dcc
[ 86.755439] r10: 0002 r9 : r8 : c7885f78
[ 86.760627] r7 : 014000c0 r6 : c7ab2308 r5 : 0001 r4 : c7ab2308
[ 86.767106] r3 : 0001 r2 : 0001 r1 : 014000c0 r0 : 0001
[ 86.773588] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 86.780672] Control: 0005317f Table: 2717c000 DAC: 0051
[ 86.786372] Process sh (pid: 1, stack limit = 0xc7884190)
[ 86.791730] Stack: (0xc7885dc0 to 0xc7886000)
[ 86.796075] 5dc0: c7885df4 c7885dd0 c0235020 c023bc04 c0728bf8 c79f1000
c7ab2308 c78c2b00
[ 86.804195] 5de0: c04f4610 c7885f78 c7885e44 c7885df8 c0236244 c0234ffc
c00a53b4 0074
[ 86.812315] 5e00: 00107000 c7885ea8 c7885e64 c05d604b c717b420 c05b4aa8
081f 0007
[ 86.820438] 5e20: c7ab2300 c7199ea0 c79baae0 c7885f78 c7199eb0 0007
c7885e54 c7885e48
[ 86.828559] 5e40: c0236640 c0236188 c7885e74 c7885e58 c02a5834 c023663c
c7885e9c 0002
[ 86.836681] 5e60: c7bf1f50 c79baae0 c7885e84 c7885e78 c02a37b8 c02a5800
c7885e9c c7885e88
[ 86.844801] 5e80: c0128fc8 c02a37a0 c7885ed4 c7885ea0
c01281e4 c0128f8c
[ 86.852922] 5ea0: c7880b60 c01280b8 00106cf8 c7215c20
c7885f78 0007
[ 86.861045] 5ec0: c7884000 00106cf8 c7885f44 c7885ed8 c00caec0 c01280c8
081f 00107d00
[ 86.869167] 5ee0: c06d0f7c c7885fb0 00053177 1180 0178 c7885fac
c7885f04 c00091e4
[ 86.877288] 5f00: c001128c c000e088 0158 c00cb114 12bc
bec504d0 b6e8bbec
[ 86.885409] 5f20: c7215c20 c7215c20 0007 00106cf8 c7885f78
c7885f74 c7885f48
[ 86.893531] 5f40: c00cb160 c00cae94 c00e6e04 c00e6568
c7215c20 c7215c20
[ 86.901652] 5f60: 0007 00106cf8 c7885fa4 c7885f78 c00cb2dc c00cb0b0
[ 86.909773] 5f80: 0007 00106cf8 b6e8dd50 0004 c000a544
c7885fa8
[ 86.917895] 5fa0: c000a3a0 c00cb2a0 0007 00106cf8 0001 00106cf8
0007
[ 86.926015] 5fc0: 0007 00106cf8 b6e8dd50 0004 0007 0004
000e9124
[ 86.934139] 5fe0: bec50a3c b6db63d0 b6e107ac 6010 0001
[ 86.942277] [] (strlen) from []
(kobject_get_path+0x34/0xac)
[ 86.949620] [] (kobject_get_path) from []
(kobject_uevent_env+0xcc/0x4b4)
[ 86.958083] [] (kobject_uevent_env) from []
(kobject_uevent+0x14/0x18)
[ 86.966287] [] (kobject_uevent) from []
(uevent_store+0x44/0x64)
[ 86.973987] [] (uevent_store) from []
(dev_attr_store+0x28/0x34)
[ 86.981672] [] (dev_attr_store) from []
(sysfs_kf_write+0x4c/0x58)
[ 86.989525] [] (sysfs_kf_write) from []
(kernfs_fop_write+0x12c/0x1c4)
[ 86.997737] [] (kernfs_fop_write) from []
(__vfs_write+0x3c/0x11c)
[ 87.005596] [] (__vfs_write) from []
(vfs_write+0xc0/0x164)
[ 87.012855] [] (vfs_write) from [] (SyS_write+0x4c/0x8c)
[ 87.019854] [] (SyS_write) from []
(ret_fast_syscall+0x0/0x38)
[ 87.027364] Code: e92dd800 e24cb004 e1a03000 e1a02003 (e5d21000)
[ 87.033544] ---[ end trace 29af93c3c072b1f4 ]---
[ 87.039277] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x000b
This is fun because it really doesn't seem to have anything to do with
atmel-nand...
I first found that on my custom board, built around an at91sam9g35-cm, but I
managed to trigger it
on an at91sam9g35-ek board, with a 4.13.3 kernel.
NB: I couldn't trigger this with ecc-strength = 2
So, here is my configuration:
- at91sam9g35-ek board with the image
ftp://www.at91.com/pub/demo/linux4sam_5.6/linux4sam-poky-at91sam9x5ek-5.6.zip
I flashed this image a first time as is, and then I flashed only the rfs with
eccType 0xc0902405.
- Kernel 4.13.3 with the quick'n dirty patch:
--- a/arch/arm/boot/dts/at91sam9x5cm.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5cm.dtsi
@@ -56,7 +56,7 @@
cs-gpios = <&pioD 4 GPIO_ACTIVE_HIGH>;
nand-bus-width = <8>;
Re: [PATCH v2 9/9] tty/serial: atmel: Prevent a warning on suspend
On 15/09/2017 16:04, Romain Izard wrote:
> The atmel serial port driver reported the following warning on suspend:
> atmel_usart f802.serial: ttyS1: Unable to drain transmitter
>
> As the ATMEL_US_TXEMPTY status bit in ATMEL_US_CSR is always cleared
> when the transmitter is disabled, we need to know the transmitter's
> state to return the real fifo state. And as ATMEL_US_CR is write-only,
> it is necessary to save the state of the transmitter in a local
> variable, and update the variable when TXEN and TXDIS is written in
> ATMEL_US_CR.
>
> After those changes, atmel_tx_empty can return "empty" on suspend, the
> warning in uart_suspend_port disappears, and suspending is 20ms shorter
> for each enabled Atmel serial port.
>
> Signed-off-by: Romain Izard
> ---
> drivers/tty/serial/atmel_serial.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 7551cab438ff..783af6648be0 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -171,6 +171,7 @@ struct atmel_uart_port {
> boolhas_hw_timer;
> struct timer_list uart_timer;
>
> + booltx_stopped;
> boolsuspended;
> unsigned intpending;
> unsigned intpending_status;
> @@ -380,6 +381,10 @@ static int atmel_config_rs485(struct uart_port *port,
> */
> static u_int atmel_tx_empty(struct uart_port *port)
> {
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> +
> + if (atmel_port->tx_stopped)
> + return TIOCSER_TEMT;
> return (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXEMPTY) ?
> TIOCSER_TEMT :
> 0;
> @@ -485,6 +490,7 @@ static void atmel_stop_tx(struct uart_port *port)
>* is fully transmitted.
>*/
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
> + atmel_port->tx_stopped = true;
>
> /* Disable interrupts */
> atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> @@ -492,6 +498,7 @@ static void atmel_stop_tx(struct uart_port *port)
> if ((port->rs485.flags & SER_RS485_ENABLED) &&
> !(port->rs485.flags & SER_RS485_RX_DURING_TX))
> atmel_start_rx(port);
> +
> }
This line feed is not needed.
Otherwise,
Acked-by: Richard Genoud
>
> /*
> @@ -521,6 +528,7 @@ static void atmel_start_tx(struct uart_port *port)
>
> /* re-enable the transmitter */
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
> + atmel_port->tx_stopped = false;
> }
>
> /*
> @@ -1866,6 +1874,7 @@ static int atmel_startup(struct uart_port *port)
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> /* enable xmit & rcvr */
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> + atmel_port->tx_stopped = false;
>
> setup_timer(&atmel_port->uart_timer,
> atmel_uart_timer_callback,
> @@ -2122,6 +2131,7 @@ static void atmel_set_termios(struct uart_port *port,
> struct ktermios *termios,
>
> /* disable receiver and transmitter */
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
> + atmel_port->tx_stopped = true;
>
> /* mode */
> if (port->rs485.flags & SER_RS485_ENABLED) {
> @@ -2207,6 +2217,7 @@ static void atmel_set_termios(struct uart_port *port,
> struct ktermios *termios,
> atmel_uart_writel(port, ATMEL_US_BRGR, quot);
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
> + atmel_port->tx_stopped = false;
>
> /* restore interrupts */
> atmel_uart_writel(port, ATMEL_US_IER, imr);
> @@ -2450,6 +2461,7 @@ static void atmel_console_write(struct console *co,
> const char *s, u_int count)
>
> /* Make sure that tx path is actually able to send characters */
> atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
> + atmel_port->tx_stopped = false;
>
> uart_console_write(port, s, count, atmel_console_putchar);
>
> @@ -2511,6 +2523,7 @@ static int __init atmel_console_setup(struct console
> *co, char *options)
> {
> int ret;
> struct uart_port *port = &atmel_ports[co->index].uart;
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> int baud = 115200;
> int bits = 8;
> in
Re: [PATCH v2] ARM: zImage: Fix stack overflow in merge_fdt_bootargs()
2017-07-22 15:02 GMT+02:00 Rask Ingemann Lambertsen :
> This function is called very early on from head.S and currently sets up a
> stack frame of more than 1024 bytes:
>
> atags_to_fdt.c: In function ‘merge_fdt_bootargs’:
> atags_to_fdt.c:98:1: warning: the frame size of 1032 bytes is larger than
> 1024 bytes [-Wframe-larger-than=]
>
> This causes a crash and failure to boot with some combinations of kernel
> version, gcc version and dtb, such as kernel version 4.1-rc1 or 4.1.0,
> gcc version 5.4.1 20161019 (Debian 5.4.1-3) and tegra20-trimslice.dtb.
>
> With this patch, merge_fdt_bootargs() is rewritten to not use a large buffer,
> thereby solving the problem of the stack overflow.
>
> As a side effect of this rewrite, you no longer get a space added in front
> of the kernel command line when no bootargs property was found in the FDT.
>
> Signed-off-by: Rask Ingemann Lambertsen
> Tested-by: Pavel Machek
> Tested-by: Sebastian Reichel
> Fixes: d0f34a11ddab ("ARM: 7437/1: zImage: Allow DTB command line
> concatenation with ATAG_CMDLINE")
seems good to me !
Reviewed-by: Richard Genoud
> ---
>
> I have tested that this works properly when
> a) only the FDT bootargs are provided,
> b) only the ATAG command line is provided, and
> c) both the FDT bootargs and the ATAG command line are provided.
>
> Changes from v1 to v2:
> The original "bootargs" NUL terminator is changed into a space only when
> appenprop_string() succeeds. Commented by Richard Genoud.
>
> arch/arm/boot/compressed/atags_to_fdt.c | 59
> +
> 1 file changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/atags_to_fdt.c
> b/arch/arm/boot/compressed/atags_to_fdt.c
> index 9448aa0c6686..87a5fba5a28e 100644
> --- a/arch/arm/boot/compressed/atags_to_fdt.c
> +++ b/arch/arm/boot/compressed/atags_to_fdt.c
> @@ -55,6 +55,29 @@ static const void *getprop(const void *fdt, const char
> *node_path,
> return fdt_getprop(fdt, offset, property, len);
> }
>
> +static int appendprop_string(void *fdt, const char *node_path,
> +const char *property, const char *string)
> +{
> + int offset = node_offset(fdt, node_path);
> +
> + if (offset < 0)
> + return offset;
> + return fdt_appendprop_string(fdt, offset, property, string);
> +}
> +
> +static int setprop_inplace_partial(void *fdt, const char *node_path,
> + const char *property, unsigned int idx,
> + const void *val, int len)
> +{
> + int offset = node_offset(fdt, node_path);
> +
> + if (offset < 0)
> + return offset;
> + return fdt_setprop_inplace_namelen_partial(fdt, offset, property,
> + strlen(property), idx,
> + val, len);
> +}
> +
> static uint32_t get_cell_size(const void *fdt)
> {
> int len;
> @@ -66,35 +89,27 @@ static uint32_t get_cell_size(const void *fdt)
> return cell_size;
> }
>
> -static void merge_fdt_bootargs(void *fdt, const char *fdt_cmdline)
> -{
> - char cmdline[COMMAND_LINE_SIZE];
> - const char *fdt_bootargs;
> - char *ptr = cmdline;
> - int len = 0;
> -
> - /* copy the fdt command line into the buffer */
> - fdt_bootargs = getprop(fdt, "/chosen", "bootargs", &len);
> - if (fdt_bootargs)
> - if (len < COMMAND_LINE_SIZE) {
> - memcpy(ptr, fdt_bootargs, len);
> - /* len is the length of the string
> -* including the NULL terminator */
> - ptr += len - 1;
> - }
> -
> - /* and append the ATAG_CMDLINE */
> - if (fdt_cmdline) {
> - len = strlen(fdt_cmdline);
> - if (ptr - cmdline + len + 2 < COMMAND_LINE_SIZE) {
> - *ptr++ = ' ';
> - memcpy(ptr, fdt_cmdline, len);
> - ptr += len;
> - }
> - }
> - *ptr = '\0';
> -
> - setprop_string(fdt, "/chosen", "bootargs", cmdline);
> +/* This is called early on from head.S, so it can't use much stack. */
> +static void merge_fdt_bootargs(void *fdt, const char *atag_cmdline)
> +{
> + const char *fdt_bootargs;
> + int len = 0;
> +
> + /* With no ATAG command line or an empty one, there is nothing to do.
> */
> + if (!atag_cmdline || strlen(atag_cmdline) ==
Re: [PATCH] ARM: zImage: Fix stack overflow in merge_fdt_bootargs()
On 16/07/2017 23:43, Rask Ingemann Lambertsen wrote:
> This function is called very early on from head.S and currently sets up a
> stack frame of more than 1024 bytes:
>
> atags_to_fdt.c: In function ‘merge_fdt_bootargs’:
> atags_to_fdt.c:98:1: warning: the frame size of 1032 bytes is larger than
> 1024 bytes [-Wframe-larger-than=]
>
> This causes a crash and failure to boot with some combinations of kernel
> version, gcc version and dtb, such as kernel version 4.1-rc1 of 4.1.0,
> gcc version 5.4.1 20161019 (Debian 5.4.1-3) and tegra20-trimslice.dtb.
>
> With this patch, merge_fdt_bootargs() is rewritten to not use a large buffer,
> thereby solving the problem of the stack overflow.
>
> As a side effect of this rewrite, you no longer get a space added in front
> of the kernel command line when no bootargs property was found in the fdt.
>
> Signed-off-by: Rask Ingemann Lambertsen
> Fixes: d0f34a11ddab ("ARM: 7437/1: zImage: Allow DTB command line
> concatenation with ATAG_CMDLINE")
> ---
>
> I have tested that this works properly when
> a) only the FDT bootargs are provided,
> b) only the ATAG command line is provided, and
> c) both the FDT bootargs and the ATAG command line are provided.
>
> arch/arm/boot/compressed/atags_to_fdt.c | 59
> +++--
> 1 file changed, 35 insertions(+), 24 deletions(-)
Thanks for fixing that !
>
> diff --git a/arch/arm/boot/compressed/atags_to_fdt.c
> b/arch/arm/boot/compressed/atags_to_fdt.c
> index 9448aa0c6686..ede23ef2e889 100644
> --- a/arch/arm/boot/compressed/atags_to_fdt.c
> +++ b/arch/arm/boot/compressed/atags_to_fdt.c
> @@ -55,6 +55,27 @@ static const void *getprop(const void *fdt, const char
> *node_path,
> return fdt_getprop(fdt, offset, property, len);
> }
>
> +static void *getprop_w(void *fdt, const char *node_path,
> +const char *property, int *len)
> +{
> + int offset = fdt_path_offset(fdt, node_path);
> +
> + if (offset == -FDT_ERR_NOTFOUND)
> + return NULL;
> +
> + return fdt_getprop_w(fdt, offset, property, len);
> +}
> +
> +static int appendprop_string(void *fdt, const char *node_path,
> + const char *property, const char *string)
> +{
> + int offset = node_offset(fdt, node_path);
> +
> + if (offset < 0)
> + return offset;
> + return fdt_appendprop_string(fdt, offset, property, string);
> +}
> +
> static uint32_t get_cell_size(const void *fdt)
> {
> int len;
> @@ -66,35 +87,25 @@ static uint32_t get_cell_size(const void *fdt)
> return cell_size;
> }
>
> -static void merge_fdt_bootargs(void *fdt, const char *fdt_cmdline)
> -{
> - char cmdline[COMMAND_LINE_SIZE];
> - const char *fdt_bootargs;
> - char *ptr = cmdline;
> - int len = 0;
> -
> - /* copy the fdt command line into the buffer */
> - fdt_bootargs = getprop(fdt, "/chosen", "bootargs", &len);
> - if (fdt_bootargs)
> - if (len < COMMAND_LINE_SIZE) {
> - memcpy(ptr, fdt_bootargs, len);
> - /* len is the length of the string
> - * including the NULL terminator */
> - ptr += len - 1;
> - }
> -
> - /* and append the ATAG_CMDLINE */
> - if (fdt_cmdline) {
> - len = strlen(fdt_cmdline);
> - if (ptr - cmdline + len + 2 < COMMAND_LINE_SIZE) {
> - *ptr++ = ' ';
> - memcpy(ptr, fdt_cmdline, len);
> - ptr += len;
> - }
> - }
> - *ptr = '\0';
> -
> - setprop_string(fdt, "/chosen", "bootargs", cmdline);
> +/* This is called early on from head.S, so it can't use much stack. */
> +static void merge_fdt_bootargs(void *fdt, const char *atag_cmdline)
> +{
> + char *fdt_bootargs;
> + int len = 0;
> +
> + /* With no ATAG command line or an empty one, there is nothing to do. */
> + if (!atag_cmdline || strlen(atag_cmdline) == 0)
> + return;
> +
> + fdt_bootargs = getprop_w(fdt, "/chosen", "bootargs", &len);
> +
> + /* With no FDT command line or an empty one, just use the ATAG one. */
> + if (!fdt_bootargs || len <= 1) {
> + setprop_string(fdt, "/chosen", "bootargs", atag_cmdline);
> + return;
> + }
> + fdt_bootargs[len - 1] = ' ';
> + appendprop_string(fdt, "/chosen", "bootargs", atag_cmdline);
Let's say appendprop_string() fails for whatever reason, the /chosen
string won't be null terminated anymore.
Won't it cause some problems ?
> }
>
> /*
>
Richard.
Re: [PATCH 3.10 219/268] tty/serial: atmel: fix race condition (TX+DMA)
Hi Willy, You can drop this patch. There's nothing to fix on 3.10.x since the DMA TX support has been introduced in 3.12. Thanks ! 2017-06-19 20:31 GMT+02:00 Willy Tarreau : > From: Richard Genoud > > commit 31ca2c63fdc0aee725cbd4f207c1256f5deaabde upstream. > > If uart_flush_buffer() is called between atmel_tx_dma() and > atmel_complete_tx_dma(), the circular buffer has been cleared, but not > atmel_port->tx_len. > That leads to a circular buffer overflow (dumping (UART_XMIT_SIZE - > atmel_port->tx_len) bytes). > > Tested-by: Nicolas Ferre > [rg] backport to 3.12 > Signed-off-by: Richard Genoud > Signed-off-by: Greg Kroah-Hartman > Signed-off-by: Willy Tarreau > --- > drivers/tty/serial/atmel_serial.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/tty/serial/atmel_serial.c > b/drivers/tty/serial/atmel_serial.c > index 82127ac..41d1df5 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -1090,6 +1090,11 @@ static void atmel_flush_buffer(struct uart_port *port) > UART_PUT_TCR(port, 0); > atmel_port->pdc_tx.ofs = 0; > } > + /* > +* in uart_flush_buffer(), the xmit circular buffer has just > +* been cleared, so we have to reset its length accordingly. > +*/ > + sg_dma_len(&atmel_port->sg_tx) = 0; > } > > /* > -- > 2.8.0.rc2.1.gbe9624a >
[PATCH] kbuild: fix header installation under fakechroot environment
Since commit fcc8487d477a ("uapi: export all headers under uapi
directories") fakechroot make bindeb-pkg fails, mismatching files for
directories:
touch: cannot touch 'usr/include/video/uvesafb.h/.install': Not a
directory
This due to a bug in fakechroot:
when using the function $(wildcard $(srcdir)/*/.) in a makefile, under a
fakechroot environment, not only directories but also files are
returned.
To circumvent that, we are using the functions:
$(sort $(dir $(wildcard $(srcdir)/*/
And thanks to Yamada Masahiro who figured out the right
filter-out/patsubst order !
Fixes: fcc8487d477a ("uapi: export all headers under uapi directories")
Signed-off-by: Richard Genoud
---
scripts/Makefile.headersinst | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/scripts/Makefile.headersinst b/scripts/Makefile.headersinst
index ce753a408c56..c583a1e1bd3c 100644
--- a/scripts/Makefile.headersinst
+++ b/scripts/Makefile.headersinst
@@ -14,7 +14,15 @@ __headers:
include scripts/Kbuild.include
srcdir:= $(srctree)/$(obj)
-subdirs := $(patsubst $(srcdir)/%/.,%,$(wildcard $(srcdir)/*/.))
+
+# When make is run under a fakechroot environment, the function
+# $(wildcard $(srcdir)/*/.) doesn't only return directories, but also regular
+# files. So, we are using a combination of sort/dir/wildcard which works
+# with fakechroot.
+subdirs := $(patsubst $(srcdir)/%/,%,\
+$(filter-out $(srcdir)/,\
+$(sort $(dir $(wildcard $(srcdir)/*/)
+
# caller may set destination dir (when installing to asm/)
_dst := $(if $(dst),$(dst),$(obj))
Re: [PATCH] tty/serial: Kconfig: remove AVR32
On 13/06/2017 22:35, Alexandre Belloni wrote: > AVR32 is now removed from the kernel, removed the config symbol that > doesn't exist anymore. Also take the opportunity to reword the help to > include sama5. > > Signed-off-by: Alexandre Belloni Acked-by: Richard Genoud 100 lines removed with the 4 patches, that's nice ! Thanks !
Re: [PATCH] tty/serial: remove AVR32 specific access wrappers
On 13/06/2017 22:25, Alexandre Belloni wrote: > Now that AVR32 is gone, the specific wrappers for that architecture are > unnecessary and will not be compiled anymore. > > Signed-off-by: Alexandre Belloni Acked-by: Richard Genoud
Re: [PATCH v2 2/2] tty/serial: atmel: make the driver DT only
On 13/06/2017 22:24, Alexandre Belloni wrote: > Now that AVR32 is gone, platform_data are not used to initialize the driver > anymore, remove that path from the driver. Also remove the now unused > struct atmel_uart_data. > > Signed-off-by: Alexandre Belloni > --- > drivers/tty/serial/atmel_serial.c | 96 > + > include/linux/platform_data/atmel.h | 10 > 2 files changed, 33 insertions(+), 73 deletions(-) Acked-by: Richard Genoud
Re: [PATCH v2 1/2] tty/serial: atmel: remove atmel_default_console_device handling
On 13/06/2017 22:24, Alexandre Belloni wrote: > atmel_default_console_device was only used by AVR32, in particular > arch/avr32/mach-at32ap/at32ap700x.c which is now gone. Remove it from the > driver. > > Signed-off-by: Alexandre Belloni > --- > > Changes in v2: > - added an empty line as suggested by Andy > - properly based the patch on v4.12-rc1 > > drivers/tty/serial/atmel_serial.c | 43 > --- > 1 file changed, 43 deletions(-) > Acked-by: Richard Genoud Thanks !
Re: [PATCH] gpio: mvebu: change compatible string for PWM support
2017-06-09 9:42 GMT+02:00 Linus Walleij :
> On Thu, Jun 1, 2017 at 10:08 PM, Ralph Sennhauser
> wrote:
>
>> As it turns out more than just Armada 370 and XP support using GPIO
>> lines as PWM lines. For example the Armada 38x family has the same
>> hardware support. As such "marvell,armada-370-xp-gpio" for the
>> compatible string is a misnomer.
>>
>> Change the compatible string to "marvell,armada-370-gpio" before the
>> driver makes it out of the -rc stage. This also follows the practice of
>> using only the first device family supported as part of the name.
>>
>> Also update the documentation and comments in the code accordingly.
>>
>> Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
>> Signed-off-by: Ralph Sennhauser
>
> Patch applied for next with the tags.
You mean for fixes right ?
The goal was to correct "marvell,armada-370-xp-gpio" before 4.12 is out.
>
> Yours,
> Linus Walleij
Regards,
Richard
Re: [PATCHv2 1/2] gpio: mvebu: fix blink counter register selection
2017-06-09 9:37 GMT+02:00 Linus Walleij :
> On Thu, Jun 1, 2017 at 2:18 PM, Richard Genoud
> wrote:
>
>> The blink counter A was always selected because 0 was forced in the
>> blink select counter register.
>> The variable 'set' was obviously there to be used as the register value,
>> selecting the B counter when id==1 and A counter when id==0.
>>
>> Tested on clearfog-pro (Marvell 88F6828)
>>
>> Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
>> Reviewed-by: Gregory CLEMENT
>> Reviewed-by: Ralph Sennhauser
>> Signed-off-by: Richard Genoud
>
> Patch applied for fixes.
>
> It appears this will clash with patches on the development branch :(
>
> I might screw up the merges so help me check the end result
> later.
Ok, no problem !
>
> Yours,
> Linus Walleij
Thanks !
Richard.
[PATCHv2 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
If more than one gpio bank has the "pwm" property, only one will be
registered successfully, all the others will fail with:
mvebu-gpio: probe of f1018140.gpio failed with error -17
That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
What was intended is mvpwm->chip->base = -1.
Like that, the numbering will be done auto-magically
Moreover, as the region might be already occupied by another pwm, we
shouldn't force:
mvpwm->chip->base = 0
nor
mvpwm->chip->base = id * MVEBU_MAX_GPIO_PER_BANK;
Tested on clearfog-pro (Marvell 88F6828)
Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
Signed-off-by: Richard Genoud
---
drivers/gpio/gpio-mvebu.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index cdef2c78cb3b..5104b6398139 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -768,6 +768,13 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
mvpwm->chip.dev = dev;
mvpwm->chip.ops = &mvebu_pwm_ops;
mvpwm->chip.npwm = mvchip->chip.ngpio;
+ /*
+* There may already be some PWM allocated, so we can't force
+* mvpwm->chip.base to a fixed point like mvchip->chip.base.
+* So, we let pwmchip_add() do the numbering and take the next free
+* region.
+*/
+ mvpwm->chip.base = -1;
spin_lock_init(&mvpwm->lock);
[PATCHv2 0/2] gpio: mvebu: fixes for PWM/blink
These are 2 fixes for gpio-mvebu, related to the PWM support introduced
by commit 757642f9a584 ("gpio: mvebu: Add limited PWM support")
As that commit was merged in 4.12-rc1, I guess these commits are 4.12
material.
Changes from v1:
- Add tags Fixes: and Reviewed-by:
- Add some comments to motivate the mvpwm->chip->base = -1 choice
instead of mvpwm->chip->base = mvchip->chip.base
Richard Genoud (2):
gpio: mvebu: fix blink counter register selection
gpio: mvebu: fix gpio bank registration when pwm is used
drivers/gpio/gpio-mvebu.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCHv2 1/2] gpio: mvebu: fix blink counter register selection
The blink counter A was always selected because 0 was forced in the
blink select counter register.
The variable 'set' was obviously there to be used as the register value,
selecting the B counter when id==1 and A counter when id==0.
Tested on clearfog-pro (Marvell 88F6828)
Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
Reviewed-by: Gregory CLEMENT
Reviewed-by: Ralph Sennhauser
Signed-off-by: Richard Genoud
---
drivers/gpio/gpio-mvebu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 19a92efabbef..cdef2c78cb3b 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
set = U32_MAX;
else
return -EINVAL;
- writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
+ writel_relaxed(set, mvebu_gpioreg_blink_counter_select(mvchip));
mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
if (!mvpwm)
Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
2017-05-30 17:14 GMT+02:00 Ralph Sennhauser :
> Hi Richard
>
> On Tue, 30 May 2017 16:45:24 +0200
> Richard Genoud wrote:
>
>> 2017-05-30 15:16 GMT+02:00 Gregory CLEMENT
>> :
>> > Hi Richard,
>> Hi Greg !
>>
>> >
>> > On mar., mai 30 2017, Richard Genoud
>> > wrote:
>> >> If more than one gpio bank has the "pwm" property, only one will be
>> >> registered successfully, all the others will fail with:
>> >> mvebu-gpio: probe of f1018140.gpio failed with error -17
>> >>
>> >> That's because in alloc_pwms(), the chip->base (aka "int pwm"),
>> >> was not set (thus, ==0) ; and 0 is a meaningful start value in
>> >> alloc_pwm(). What was intended is chip->base = -1.
>> >> Like that, the numbering will be done auto-magically
>> >>
>> >> Tested on clearfog-pro (Marvell 88F6828)
>> >>
>> >> Signed-off-by: Richard Genoud
>> >> ---
>> >> drivers/gpio/gpio-mvebu.c | 1 +
>> >> 1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> >> index cdef2c78cb3b..4734923e11fd 100644
>> >> --- a/drivers/gpio/gpio-mvebu.c
>> >> +++ b/drivers/gpio/gpio-mvebu.c
>> >> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct
>> >> platform_device *pdev, mvpwm->chip.dev = dev;
>> >> mvpwm->chip.ops = &mvebu_pwm_ops;
>> >> mvpwm->chip.npwm = mvchip->chip.ngpio;
>> >> + mvpwm->chip.base = -1;
>> >
>> > Why not using
>> > mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
>> > as it is done in the mvebu_gpio_probe() function?
>> Yes, that was my first move:
>> mvpwm->chip.base = mvchip->chip.base;
>>
>> But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
>> the PWM base,
>> (mvpwm->chip is a struct pwm_chip), so it would we weird to have
>> "holes" in the declared PWMs.
>> I'm not clear, so here's an example:
>> If, in the DTS, we have:
>> gpio0: gpio@18100 {
>> compatible = "marvell,armada-370-xp-gpio",
>> "marvell,orion-gpio";
>> reg = <0x18100 0x40>, <0x181c0 0x08>;
>> reg-names = "gpio"; /* "pwm" missing */
>> [...]
>> gpio1: gpio@18140 {
>> compatible = "marvell,armada-370-xp-gpio",
>> "marvell,orion-gpio";
>> reg = <0x18140 0x40>, <0x181c8 0x08>;
>> reg-names = "gpio", "pwm";
>> In this case, if gpio0 is not declared as PWM capable, the PWM
>> numbering will start at 32 if we have
>> mvpwm->chip.base = mvchip->chip.base;
>> but it will start at 0 if we have mvpwm->chip.base = -1;
>>
>> The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
>> the stable numbering:
>> if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
>> sysfs will stay the same.
>> And if we have mvpwm->chip.base = -1; the pwm numbering will be
>> shifted.
>>
>> Looking back at the V5 of this patch
>> https://www.spinics.net/lists/kernel/msg2484889.html
>> There was the line:
>> mvpwm->chip.base = mvchip->chip.base;
>> I guess it got lost in the v6 rebase.
>>
>> So I could change it back, but I'm not sure which one is better.
>
> Thierry Redding pointed out that the region might already be occupied
> by another PWM chip, unlikely but not impossible. That's why it got
> changed for v6.
>
> See https://www.spinics.net/lists/devicetree/msg173138.html
Hum, I see.
But still, if mvpwm->chip.base is 0, we will have an error in alloc_pwms():
in pmvebu_pwm_probe(), mvpwm->chip.base is set to 0 (kzalloc), then
pwmchip_add(&mvpwm->chip) is called,
which will call alloc_pwms(chip->base/* == 0 */, chip->npwm);
The 1st time (1st gpio bank), it will be ok because:
start = bitmap_find_next_zero_area(allocated_pwms, MAX_PWMS, from, count, 0);
if (pwm >= 0 && start != pwm)
return -EEXIST;
start is 0, pwm is 0, everything is good.
BUT, on the second time (for gpio1), we will have start = 32 and pwm=0
=> -EEXIST !
So there's something to fix.
And It seems that setting mvpwm->chip.base to -1 is a good choice
since it doesn't
force the numbering to something already taken.
Regards;
Richard.
Re: [PATCH v6 1/4] gpio: mvebu: Add limited PWM support
2017-05-30 16:51 GMT+02:00 Ralph Sennhauser : > On Tue, 30 May 2017 15:40:00 +0200 > Gregory CLEMENT wrote: > >> Hi Ralph and Linus, >> >> I resurrect this thread following the series post by Richard Genoud: >> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-May/509461.html >> >> On lun., avril 24 2017, Linus Walleij >> wrote: >> >> > On Fri, Apr 14, 2017 at 5:40 PM, Ralph Sennhauser >> > wrote: >> > >> >> From: Andrew Lunn >> >> >> >> Armada 370/XP devices can 'blink' GPIO lines with a configurable on >> >> and off period. This can be modelled as a PWM. >> >> >> >> However, there are only two sets of PWM configuration registers for >> >> all the GPIO lines. This driver simply allows a single GPIO line >> >> per GPIO chip of 32 lines to be used as a PWM. Attempts to use >> >> more return EBUSY. >> >> >> >> Due to the interleaving of registers it is not simple to separate >> >> the PWM driver from the GPIO driver. Thus the GPIO driver has been >> >> extended with a PWM driver. >> >> >> >> Signed-off-by: Andrew Lunn >> >> URL: https://patchwork.ozlabs.org/patch/427287/ >> >> URL: https://patchwork.ozlabs.org/patch/427295/ >> >> [Ralph Sennhauser: >> >> * Port forward >> >> * Merge PWM portion into gpio-mvebu.c >> >> * Switch to atomic PWM API >> >> * Add new compatible string marvell,armada-370-xp-gpio >> >> The fact that Richard managed to run the code on Armada 38x draw my >> attention on the documentation that need to be amend. > > Hi Gregory > > But does it behave as a pwm-fan? I don't have access to the 38x > functional spec (no NDA with Marvell), the one for XP is public. > According to Andrew the 38x doesn't have the blinking hardware and so I > expect the pwm-fan is just an overblown gpio-fan there, additionally > writing some random values to random registers where luck has it nothing > goes wrong. > > Kirkwood apparently has blinking support but only at fixed frequency of > 10Hz, so not usable for this purpose either. > > So to my knowledge it really is just 370 and XP. Hi Ralph, I have the functional spec (no NDA needed, but it's not the full one) : A38x-Functional-Spec-PU0A.pdf https://marvellcorp.wufoo.com/forms/marvell-armada-38x-functional-specifications/ (just an email needed, no blood signing nor chicken slaughtering) There are the GPIO Blink Counter A/B is ON/OFF Duration Registers as well as the Blink Enable Registers. I've done a pwm with different periods (8ms, 4ms, 100ns). Looking at the scope, it seems to work pretty well :) >> >> And it makes me realized that I missed the bad naming of the >> compatible string. We don't use family name for the compatible >> string, but the name of the first SoC compatible with. So in this >> case we should use "marvell,armada-370", as it is still in rc and not >> yet deployed. What about fixing the name now? > > I don't mind but certainly not for me to decide at this point. > >> >> I can send a patch for it if you want and fix the dts that are still >> in my mvebu branch. > > Sure, or if you prefer I can do it as well if we decide for the change. > No preference. > > Ralph
Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
2017-05-30 15:16 GMT+02:00 Gregory CLEMENT :
> Hi Richard,
Hi Greg !
>
> On mar., mai 30 2017, Richard Genoud wrote:
>
>> If more than one gpio bank has the "pwm" property, only one will be
>> registered successfully, all the others will fail with:
>> mvebu-gpio: probe of f1018140.gpio failed with error -17
>>
>> That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
>> set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
>> What was intended is chip->base = -1.
>> Like that, the numbering will be done auto-magically
>>
>> Tested on clearfog-pro (Marvell 88F6828)
>>
>> Signed-off-by: Richard Genoud
>> ---
>> drivers/gpio/gpio-mvebu.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> index cdef2c78cb3b..4734923e11fd 100644
>> --- a/drivers/gpio/gpio-mvebu.c
>> +++ b/drivers/gpio/gpio-mvebu.c
>> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>> mvpwm->chip.dev = dev;
>> mvpwm->chip.ops = &mvebu_pwm_ops;
>> mvpwm->chip.npwm = mvchip->chip.ngpio;
>> + mvpwm->chip.base = -1;
>
> Why not using
> mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
> as it is done in the mvebu_gpio_probe() function?
Yes, that was my first move:
mvpwm->chip.base = mvchip->chip.base;
But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
the PWM base,
(mvpwm->chip is a struct pwm_chip), so it would we weird to have
"holes" in the declared PWMs.
I'm not clear, so here's an example:
If, in the DTS, we have:
gpio0: gpio@18100 {
compatible = "marvell,armada-370-xp-gpio",
"marvell,orion-gpio";
reg = <0x18100 0x40>, <0x181c0 0x08>;
reg-names = "gpio"; /* "pwm" missing */
[...]
gpio1: gpio@18140 {
compatible = "marvell,armada-370-xp-gpio",
"marvell,orion-gpio";
reg = <0x18140 0x40>, <0x181c8 0x08>;
reg-names = "gpio", "pwm";
In this case, if gpio0 is not declared as PWM capable, the PWM
numbering will start at 32 if we have
mvpwm->chip.base = mvchip->chip.base;
but it will start at 0 if we have mvpwm->chip.base = -1;
The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
the stable numbering:
if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
sysfs will stay the same.
And if we have mvpwm->chip.base = -1; the pwm numbering will be shifted.
Looking back at the V5 of this patch
https://www.spinics.net/lists/kernel/msg2484889.html
There was the line:
mvpwm->chip.base = mvchip->chip.base;
I guess it got lost in the v6 rebase.
So I could change it back, but I'm not sure which one is better.
>
> I think that if you use base = -1, then the number start from (512 -
> number of pin already use). So starting from a low number for one
> compatible and a high number for an other compatible could be confusing.
>
> Besides that I agree that mvpwm->chip.base must be initialized and here
> again for adding mor context to this patch, we could add:
>
> Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
yes, definitely !
should I resend the patch with it or the maintainer will add it ?
> Gregory
>
>>
>> spin_lock_init(&mvpwm->lock);
>>
>
> --
> Gregory Clement, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
[PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
If more than one gpio bank has the "pwm" property, only one will be registered successfully, all the others will fail with: mvebu-gpio: probe of f1018140.gpio failed with error -17 That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm(). What was intended is chip->base = -1. Like that, the numbering will be done auto-magically Tested on clearfog-pro (Marvell 88F6828) Signed-off-by: Richard Genoud --- drivers/gpio/gpio-mvebu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c index cdef2c78cb3b..4734923e11fd 100644 --- a/drivers/gpio/gpio-mvebu.c +++ b/drivers/gpio/gpio-mvebu.c @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev, mvpwm->chip.dev = dev; mvpwm->chip.ops = &mvebu_pwm_ops; mvpwm->chip.npwm = mvchip->chip.ngpio; + mvpwm->chip.base = -1; spin_lock_init(&mvpwm->lock);
[PATCH 1/2] gpio: mvebu: fix blink counter register selection
The blink counter A was always selected because 0 was forced in the blink select counter register. The variable 'set' was obviously there to be used as the register value, selecting the B counter when id==1 and A counter when id==0. Tested on clearfog-pro (Marvell 88F6828) Signed-off-by: Richard Genoud --- drivers/gpio/gpio-mvebu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c index 19a92efabbef..cdef2c78cb3b 100644 --- a/drivers/gpio/gpio-mvebu.c +++ b/drivers/gpio/gpio-mvebu.c @@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev, set = U32_MAX; else return -EINVAL; - writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip)); + writel_relaxed(set, mvebu_gpioreg_blink_counter_select(mvchip)); mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL); if (!mvpwm)
Re: [PATCH v2] tty/serial: atmel: use offset_in_page() macro
On 29/04/2017 03:39, Geliang Tang wrote: > Use offset_in_page() macro instead of open-coding. > > Signed-off-by: Geliang Tang Acked-by: Richard Genoud > --- > Changes in v2: > - include mm.h > --- > drivers/tty/serial/atmel_serial.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/atmel_serial.c > b/drivers/tty/serial/atmel_serial.c > index c355ac9..d25f044 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -46,6 +46,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -959,7 +960,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port) > sg_set_page(&atmel_port->sg_tx, > virt_to_page(port->state->xmit.buf), > UART_XMIT_SIZE, > - (unsigned long)port->state->xmit.buf & ~PAGE_MASK); > + offset_in_page(port->state->xmit.buf)); > nent = dma_map_sg(port->dev, > &atmel_port->sg_tx, > 1, > @@ -1141,7 +1142,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port) > sg_set_page(&atmel_port->sg_rx, > virt_to_page(ring->buf), > sizeof(struct atmel_uart_char) * ATMEL_SERIAL_RINGSIZE, > - (unsigned long)ring->buf & ~PAGE_MASK); > + offset_in_page(ring->buf)); > nent = dma_map_sg(port->dev, > &atmel_port->sg_rx, > 1, > Thanks !
Re: [PATCH] serial: sh-sci: Fix race condition causing garbage during shutdown
On 25/04/2017 20:15, Geert Uytterhoeven wrote:
> If DMA is enabled and used, a burst of old data may be seen on the
> serial console during "poweroff" or "reboot". uart_flush_buffer()
> clears the circular buffer, but sci_port.tx_dma_len is not reset.
> This leads to a circular buffer overflow, dumping (UART_XMIT_SIZE -
> sci_port.tx_dma_len) bytes.
>
> To fix this, add a .flush_buffer() callback that resets
> sci_port.tx_dma_len.
>
> Inspired by commit 31ca2c63fdc0aee7 ("tty/serial: atmel: fix race
> condition (TX+DMA)").
>
> Signed-off-by: Geert Uytterhoeven
> ---
> A big thanks to Richard!
> I had no idea what was happening here, until I saw his patch passing by
> in a stable backport.
:)
you're welcome !
> In v4.3 and older, the field is called sg_len_tx.
> ---
> drivers/tty/serial/sh-sci.c | 16 ++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 9a47cc4f16a2798f..a15739202d6c75f2 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1545,7 +1545,16 @@ static void sci_free_dma(struct uart_port *port)
> if (s->chan_rx)
> sci_rx_dma_release(s, false);
> }
> -#else
> +
> +static void sci_flush_buffer(struct uart_port *port)
> +{
> + /*
> + * In uart_flush_buffer(), the xmit circular buffer has just been
> + * cleared, so we have to reset tx_dma_len accordingly.
> + */
> + to_sci_port(port)->tx_dma_len = 0;
> +}
> +#else /* !CONFIG_SERIAL_SH_SCI_DMA */
> static inline void sci_request_dma(struct uart_port *port)
> {
> }
> @@ -1553,7 +1562,9 @@ static inline void sci_request_dma(struct uart_port
> *port)
> static inline void sci_free_dma(struct uart_port *port)
> {
> }
> -#endif
> +
> +#define sci_flush_buffer NULL
> +#endif /* !CONFIG_SERIAL_SH_SCI_DMA */
>
> static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
> {
> @@ -2566,6 +2577,7 @@ static const struct uart_ops sci_uart_ops = {
> .break_ctl = sci_break_ctl,
> .startup= sci_startup,
> .shutdown = sci_shutdown,
> + .flush_buffer = sci_flush_buffer,
> .set_termios= sci_set_termios,
> .pm = sci_pm,
> .type = sci_type,
>
Re: [PATCH] tty/serial: atmel: use offset_in_page() macro
On 22/04/2017 03:21, Geliang Tang wrote: > Use offset_in_page() macro instead of open-coding. > > Signed-off-by: Geliang Tang > --- > drivers/tty/serial/atmel_serial.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/atmel_serial.c > b/drivers/tty/serial/atmel_serial.c > index c355ac9..f398db8 100644 > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -959,7 +959,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port) > sg_set_page(&atmel_port->sg_tx, > virt_to_page(port->state->xmit.buf), > UART_XMIT_SIZE, > - (unsigned long)port->state->xmit.buf & ~PAGE_MASK); > + offset_in_page(port->state->xmit.buf)); > nent = dma_map_sg(port->dev, > &atmel_port->sg_tx, > 1, > @@ -1141,7 +1141,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port) > sg_set_page(&atmel_port->sg_rx, > virt_to_page(ring->buf), > sizeof(struct atmel_uart_char) * ATMEL_SERIAL_RINGSIZE, > - (unsigned long)ring->buf & ~PAGE_MASK); > + offset_in_page(ring->buf)); > nent = dma_map_sg(port->dev, > &atmel_port->sg_rx, > 1, > I guess you want to #include (even if it compiles ok without it) Regards, Richard.
Re: [PATCH 3.18 049/124] tty/serial: atmel: fix race condition (TX+DMA)
On 20/04/2017 08:35, Greg Kroah-Hartman wrote: > 3.18-stable review patch. If anyone has any objections, please let me know. > > -- > > From: Richard Genoud > > commit 31ca2c63fdc0aee725cbd4f207c1256f5deaabde upstream. > > If uart_flush_buffer() is called between atmel_tx_dma() and > atmel_complete_tx_dma(), the circular buffer has been cleared, but not > atmel_port->tx_len. > That leads to a circular buffer overflow (dumping (UART_XMIT_SIZE - > atmel_port->tx_len) bytes). > > Tested-by: Nicolas Ferre > Signed-off-by: Richard Genoud > Signed-off-by: Greg Kroah-Hartman > > --- > drivers/tty/serial/atmel_serial.c |5 + > 1 file changed, 5 insertions(+) > > --- a/drivers/tty/serial/atmel_serial.c > +++ b/drivers/tty/serial/atmel_serial.c > @@ -1867,6 +1867,11 @@ static void atmel_flush_buffer(struct ua > UART_PUT_TCR(port, 0); > atmel_port->pdc_tx.ofs = 0; > } > + /* > + * in uart_flush_buffer(), the xmit circular buffer has just > + * been cleared, so we have to reset tx_len accordingly. > + */ > + atmel_port->tx_len = 0; > } > > /* > > Hi, This won't compile on 3.18 kernel since ->tx_len doesn't exist yet. Here is the backported version of this patch: Thanks ! == commit 31ca2c63fdc0aee725cbd4f207c1256f5deaabde upstream. If uart_flush_buffer() is called between atmel_tx_dma() and atmel_complete_tx_dma(), the circular buffer has been cleared, but not its length. That leads to a circular buffer overflow (dumping (UART_XMIT_SIZE - sg_dma_len(&atmel_port->sg_tx)) bytes). Signed-off-by: Richard Genoud --- drivers/tty/serial/atmel_serial.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index daaed7c79e4f..a10c778be2ad 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -1867,6 +1867,11 @@ static void atmel_flush_buffer(struct uart_port *port) UART_PUT_TCR(port, 0); atmel_port->pdc_tx.ofs = 0; } + /* +* in uart_flush_buffer(), the xmit circular buffer has just +* been cleared, so we have to reset its length accordingly. +*/ + sg_dma_len(&atmel_port->sg_tx) = 0; } /*
[PATCHv3 2/2] drm/panel: simple: Add support for Winstar WF35LTIACD
This adds support for the Winstar Display Co. WF35LTIACD 3.5" QVGA TFT
LCD panel, which can be supported by the simple panel driver.
Acked-by: Rob Herring
Signed-off-by: Richard Genoud
---
Changes since v2:
Remove status, reg and unit-addresses from example
Changes since v1:
Add power-supply property and an example in documentation
.../bindings/display/panel/winstar,wf35ltiacd | 48 ++
drivers/gpu/drm/panel/panel-simple.c | 28 +
2 files changed, 76 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
diff --git a/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
new file mode 100644
index ..2a7e6e3ba64c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
@@ -0,0 +1,48 @@
+Winstar Display Corporation 3.5" QVGA (320x240) TFT LCD panel
+
+Required properties:
+- compatible: should be "winstar,wf35ltiacd"
+- power-supply: regulator to provide the VCC supply voltage (3.3 volts)
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
+
+Example:
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&hlcdc_pwm 0 5 PWM_POLARITY_INVERTED>;
+ brightness-levels = <0 31 63 95 127 159 191 223 255>;
+ default-brightness-level = <191>;
+ power-supply = <&bl_reg>;
+ };
+
+ bl_reg: backlight_regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "backlight-power-supply";
+ regulator-min-microvolt = <500>;
+ regulator-max-microvolt = <500>;
+ };
+
+ panel: panel {
+ compatible = "winstar,wf35ltiacd", "simple-panel";
+ backlight = <&backlight>;
+ power-supply = <&panel_reg>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ panel_input: endpoint {
+ remote-endpoint = <&hlcdc_panel_output>;
+ };
+ };
+ };
+
+ panel_reg: panel_regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "panel-power-supply";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ };
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 06aaf79de8c8..5d45664c6c0e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1713,6 +1713,31 @@ static const struct panel_desc urt_umsh_8596md_parallel
= {
.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
};
+static const struct drm_display_mode winstar_wf35ltiacd_mode = {
+ .clock = 6410,
+ .hdisplay = 320,
+ .hsync_start = 320 + 20,
+ .hsync_end = 320 + 20 + 30,
+ .htotal = 320 + 20 + 30 + 38,
+ .vdisplay = 240,
+ .vsync_start = 240 + 4,
+ .vsync_end = 240 + 4 + 3,
+ .vtotal = 240 + 4 + 3 + 15,
+ .vrefresh = 60,
+ .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static const struct panel_desc winstar_wf35ltiacd = {
+ .modes = &winstar_wf35ltiacd_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 70,
+ .height = 53,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+};
+
static const struct of_device_id platform_of_match[] = {
{
.compatible = "ampire,am800480r3tmqwa1h",
@@ -1892,6 +1917,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "urt,umsh-8596md-20t",
.data = &urt_umsh_8596md_parallel,
}, {
+ .compatible = "winstar,wf35ltiacd",
+ .data = &winstar_wf35ltiacd,
+ }, {
/* sentinel */
}
};
Re: [PATCHv2 2/2] drm/panel: simple: Add support for Winstar WF35LTIACD
2017-03-24 16:58 GMT+01:00 Rob Herring :
> On Mon, Mar 20, 2017 at 02:32:22PM +0100, Richard Genoud wrote:
>> This adds support for the Winstar Display Co. WF35LTIACD 3.5" QVGA TFT
>> LCD panel, which can be supported by the simple panel driver.
>>
>> Signed-off-by: Richard Genoud
>> ---
>>
>> Changes since v1:
>> Add power-supply property and an example in documentation
>>
>> .../bindings/display/panel/winstar,wf35ltiacd | 53
>> ++
>> drivers/gpu/drm/panel/panel-simple.c | 28
>> 2 files changed, 81 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
>> b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
>> new file mode 100644
>> index ..9ff59b868b28
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
>> @@ -0,0 +1,53 @@
>> +Winstar Display Corporation 3.5" QVGA (320x240) TFT LCD panel
>> +
>> +Required properties:
>> +- compatible: should be "winstar,wf35ltiacd"
>> +- power-supply: regulator to provide the VCC supply voltage (3.3 volts)
>> +
>> +This binding is compatible with the simple-panel binding, which is specified
>> +in simple-panel.txt in this directory.
>> +
>> +Example:
>> + backlight: backlight {
>> + compatible = "pwm-backlight";
>> + pwms = <&hlcdc_pwm 0 5 PWM_POLARITY_INVERTED>;
>> + brightness-levels = <0 31 63 95 127 159 191 223 255>;
>> + default-brightness-level = <191>;
>> + power-supply = <&bl_reg>;
>> + status = "okay";
>
> Drop status from examples.
>
>> + };
>> +
>> + bl_reg: backlight_regulator {
>> + compatible = "regulator-fixed";
>> + regulator-name = "backlight-power-supply";
>> + regulator-min-microvolt = <500>;
>> + regulator-max-microvolt = <500>;
>> + status = "okay";
>> + };
>> +
>> + panel: panel {
>> + compatible = "winstar,wf35ltiacd", "simple-panel";
>> + backlight = <&backlight>;
>> + power-supply = <&panel_reg>;
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + status = "okay";
>> +
>> + port@0 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + panel_input: endpoint@0 {
>> + reg = <0>;
>
> You don't need reg and unit-addresses for these.
>
> With that,
>
> Acked-by: Rob Herring
>
Ok, I'll resend the patch with reg and unit-addresses removed.
Thanks !
>> + remote-endpoint = <&hlcdc_panel_output>;
>> + };
>> + };
>> + };
>> +
>> + panel_reg: panel_regulator {
>> + compatible = "regulator-fixed";
>> + regulator-name = "panel-power-supply";
>> + regulator-min-microvolt = <330>;
>> + regulator-max-microvolt = <330>;
>> + status = "okay";
>> + };
[PATCHv2 2/2] drm/panel: simple: Add support for Winstar WF35LTIACD
This adds support for the Winstar Display Co. WF35LTIACD 3.5" QVGA TFT
LCD panel, which can be supported by the simple panel driver.
Signed-off-by: Richard Genoud
---
Changes since v1:
Add power-supply property and an example in documentation
.../bindings/display/panel/winstar,wf35ltiacd | 53 ++
drivers/gpu/drm/panel/panel-simple.c | 28
2 files changed, 81 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
diff --git a/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
new file mode 100644
index ..9ff59b868b28
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
@@ -0,0 +1,53 @@
+Winstar Display Corporation 3.5" QVGA (320x240) TFT LCD panel
+
+Required properties:
+- compatible: should be "winstar,wf35ltiacd"
+- power-supply: regulator to provide the VCC supply voltage (3.3 volts)
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
+
+Example:
+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&hlcdc_pwm 0 5 PWM_POLARITY_INVERTED>;
+ brightness-levels = <0 31 63 95 127 159 191 223 255>;
+ default-brightness-level = <191>;
+ power-supply = <&bl_reg>;
+ status = "okay";
+ };
+
+ bl_reg: backlight_regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "backlight-power-supply";
+ regulator-min-microvolt = <500>;
+ regulator-max-microvolt = <500>;
+ status = "okay";
+ };
+
+ panel: panel {
+ compatible = "winstar,wf35ltiacd", "simple-panel";
+ backlight = <&backlight>;
+ power-supply = <&panel_reg>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ panel_input: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&hlcdc_panel_output>;
+ };
+ };
+ };
+
+ panel_reg: panel_regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "panel-power-supply";
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ status = "okay";
+ };
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 06aaf79de8c8..5d45664c6c0e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1713,6 +1713,31 @@ static const struct panel_desc urt_umsh_8596md_parallel
= {
.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
};
+static const struct drm_display_mode winstar_wf35ltiacd_mode = {
+ .clock = 6410,
+ .hdisplay = 320,
+ .hsync_start = 320 + 20,
+ .hsync_end = 320 + 20 + 30,
+ .htotal = 320 + 20 + 30 + 38,
+ .vdisplay = 240,
+ .vsync_start = 240 + 4,
+ .vsync_end = 240 + 4 + 3,
+ .vtotal = 240 + 4 + 3 + 15,
+ .vrefresh = 60,
+ .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static const struct panel_desc winstar_wf35ltiacd = {
+ .modes = &winstar_wf35ltiacd_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 70,
+ .height = 53,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+};
+
static const struct of_device_id platform_of_match[] = {
{
.compatible = "ampire,am800480r3tmqwa1h",
@@ -1892,6 +1917,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "urt,umsh-8596md-20t",
.data = &urt_umsh_8596md_parallel,
}, {
+ .compatible = "winstar,wf35ltiacd",
+ .data = &winstar_wf35ltiacd,
+ }, {
/* sentinel */
}
};
[PATCH] tty/serial: atmel: fix race condition (TX+DMA)
If uart_flush_buffer() is called between atmel_tx_dma() and atmel_complete_tx_dma(), the circular buffer has been cleared, but not atmel_port->tx_len. That leads to a circular buffer overflow (dumping (UART_XMIT_SIZE - atmel_port->tx_len) bytes). Tested-by: Nicolas Ferre Signed-off-by: Richard Genoud Cc: stable # 3.12+ --- NB: this will apply on kernels 4.3+, for kernels before 4.3, the code will be: sg_dma_len(&atmel_port->sg_tx) = 0; drivers/tty/serial/atmel_serial.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c index d9c05e05d896..5c23fb4e552b 100644 --- a/drivers/tty/serial/atmel_serial.c +++ b/drivers/tty/serial/atmel_serial.c @@ -1954,6 +1954,11 @@ static void atmel_flush_buffer(struct uart_port *port) atmel_uart_writel(port, ATMEL_PDC_TCR, 0); atmel_port->pdc_tx.ofs = 0; } + /* +* in uart_flush_buffer(), the xmit circular buffer has just +* been cleared, so we have to reset tx_len accordingly. +*/ + atmel_port->tx_len = 0; } /*
Re: [RFC PATCH] tty/serial: atmel: fix TX path in atmel_console_write()
On 15/03/2017 16:29, Nicolas Ferre wrote:
> A side effect of 89d8232411a8 ("tty/serial: atmel_serial: BUG: stop DMA
> from transmitting in stop_tx") is that the console can be called with
> TX path disabled. Then the system would hang trying to push charecters
> out in atmel_console_putchar().
>
> Signed-off-by: Nicolas Ferre
> Fixes: 89d8232411a8 ("tty/serial: atmel_serial: BUG: stop DMA from
> transmitting
> in stop_tx")
> Cc: stable # 4.4+
> ---
> Hi Richard,
>
> I found this to fix the problem with system hang in my linux-4.4-at91 branch
> (in the atmel_console_putchar() waiting loop actually). I'm open to more
> insignt.
> As we cannot figure out if this bit is set or not, I didn't preserve the
> current status...
>
> Regards,
>
> drivers/tty/serial/atmel_serial.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index dcebb28ffbc4..7372dbdb7a4c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2483,6 +2483,9 @@ static void atmel_console_write(struct console *co,
> const char *s, u_int count)
> pdc_tx = atmel_uart_readl(port, ATMEL_PDC_PTSR) & ATMEL_PDC_TXTEN;
> atmel_uart_writel(port, ATMEL_PDC_PTCR, ATMEL_PDC_TXTDIS);
>
> + /* Make sure that tx path is actually able to send characters */
> + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
> +
> uart_console_write(port, s, count, atmel_console_putchar);
>
> /*
>
Acked-by: Richard Genoud
Re: [RFC PATCH] tty/serial: atmel: fix TX path in atmel_console_write()
2017-03-15 17:56 GMT+01:00 Nicolas Ferre :
> Le 15/03/2017 à 17:19, Richard Genoud a écrit :
>> On 15/03/2017 16:29, Nicolas Ferre wrote:
>>> A side effect of 89d8232411a8 ("tty/serial: atmel_serial: BUG: stop DMA
>>> from transmitting in stop_tx") is that the console can be called with
>>> TX path disabled. Then the system would hang trying to push charecters
>>> out in atmel_console_putchar().
>>>
>>> Signed-off-by: Nicolas Ferre
>>> Fixes: 89d8232411a8 ("tty/serial: atmel_serial: BUG: stop DMA from
>>> transmitting
>>> in stop_tx")
>>> Cc: stable # 4.4+
>>> ---
>>> Hi Richard,
>>>
>>> I found this to fix the problem with system hang in my linux-4.4-at91 branch
>>> (in the atmel_console_putchar() waiting loop actually). I'm open to more
>>> insignt.
>>> As we cannot figure out if this bit is set or not, I didn't preserve the
>>> current status...
>>>
>>> Regards,
>>
>> So, I'm guessing that you may see some lines/characters printed twice on
>> the screen, don't you ?
>
> Well, actually, I don't think so because the repetitions that I see are
> probably due to open/close/open/close/re-open/... of the serial console
> itself.
>
> Same with the line "random: udevd: uninitialized urandom read (16 bytes
> read, 21 bits of entropy available)", they happen at different moment in
> time => the printk log timestamping seem to indicate that they are
> different.
Hi Nicolas,
It seems that the problem is between atmel_tx_dma() and its callback
atmel_complete_tx_dma().
At some point, atmel_tx_dma() is called, does the job, and then, just
before the callback is called, the xmit->head and xmit->tail pointers
are set to zero (by uart_flush_buffer())
So, when atmel_complete_tx_dma() is called, it does:
xmit->tail += atmel_port->tx_len;
not knowing that the head and tail pointers have been reseted.
=> it's like there's (UART_XMIT_SIZE - atmel_port->tx_len) characters to
transmit on the serial line.
PS: I can trigger this bug by holding down the d key at login and then
ctrl - basically, a ctrl-d just after sending text - with a rate success
of about 1/5 :)
Could you try this patch to see if it corrects also your system hang ?
(The patch is small, but the bug hunt was a headache :))
[PATCH] tty/serial: atmel: fix race condition (TX+DMA)
If uart_flush_buffer() is called between atmel_tx_dma() and
atmel_complete_tx_dma(), the circular buffer has been cleared, but not
atmel_port->tx_len.
That leads to a circular buffer overflow (dumping (UART_XMIT_SIZE -
atmel_port->tx_len) bytes).
Signed-off-by: Richard Genoud
---
drivers/tty/serial/atmel_serial.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index 833d3d80446f..89552157e334 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -1934,6 +1934,11 @@ static void atmel_flush_buffer(struct uart_port
*port)
atmel_uart_writel(port, ATMEL_PDC_TCR, 0);
atmel_port->pdc_tx.ofs = 0;
}
+ /*
+* in uart_flush_buffer(), the xmit circular buffer has just
+* been cleared, so we have to reset tx_len accordingly.
+*/
+ atmel_port->tx_len = 0;
}
/*
Re: [RFC PATCH] tty/serial: atmel: fix TX path in atmel_console_write()
On 15/03/2017 16:29, Nicolas Ferre wrote:
> A side effect of 89d8232411a8 ("tty/serial: atmel_serial: BUG: stop DMA
> from transmitting in stop_tx") is that the console can be called with
> TX path disabled. Then the system would hang trying to push charecters
> out in atmel_console_putchar().
>
> Signed-off-by: Nicolas Ferre
> Fixes: 89d8232411a8 ("tty/serial: atmel_serial: BUG: stop DMA from
> transmitting
> in stop_tx")
> Cc: stable # 4.4+
> ---
> Hi Richard,
>
> I found this to fix the problem with system hang in my linux-4.4-at91 branch
> (in the atmel_console_putchar() waiting loop actually). I'm open to more
> insignt.
> As we cannot figure out if this bit is set or not, I didn't preserve the
> current status...
>
> Regards,
So, I'm guessing that you may see some lines/characters printed twice on
the screen, don't you ?
> drivers/tty/serial/atmel_serial.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index dcebb28ffbc4..7372dbdb7a4c 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -2483,6 +2483,9 @@ static void atmel_console_write(struct console *co,
> const char *s, u_int count)
> pdc_tx = atmel_uart_readl(port, ATMEL_PDC_PTSR) & ATMEL_PDC_TXTEN;
> atmel_uart_writel(port, ATMEL_PDC_PTCR, ATMEL_PDC_TXTDIS);
>
> + /* Make sure that tx path is actually able to send characters */
> + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
> +
> uart_console_write(port, s, count, atmel_console_putchar);
>
> /*
>
Richard.
Re: [PATCH] tty/serial: atmel_serial: BUG: stop DMA from transmitting in stop_tx
2017-03-15 12:37 GMT+01:00 Nicolas Ferre : > Le 13/12/2016 à 17:27, Richard Genoud a écrit : >> If we don't disable the transmitter in atmel_stop_tx, the DMA buffer >> continues to send data until it is emptied. >> This cause problems with the flow control (CTS is asserted and data are >> still sent). >> >> So, disabling the transmitter in atmel_stop_tx is a sane thing to do. >> >> Tested on at91sam9g35-cm(DMA) >> Tested for regressions on sama5d2-xplained(Fifo) and at91sam9g20ek(PDC) >> >> Cc: (beware, this won't apply before 4.3) >> Signed-off-by: Richard Genoud >> --- >> drivers/tty/serial/atmel_serial.c | 11 +++ >> 1 file changed, 11 insertions(+) >> >> NB: this is not for the 4.10 merge window, I'm just sending it now to >> have some comments if someone is againts it. >> >> diff --git a/drivers/tty/serial/atmel_serial.c >> b/drivers/tty/serial/atmel_serial.c >> index 168b10cad47b..f9d42de5ab2d 100644 >> --- a/drivers/tty/serial/atmel_serial.c >> +++ b/drivers/tty/serial/atmel_serial.c >> @@ -481,6 +481,14 @@ static void atmel_stop_tx(struct uart_port *port) >> /* disable PDC transmit */ >> atmel_uart_writel(port, ATMEL_PDC_PTCR, ATMEL_PDC_TXTDIS); >> } >> + >> + /* >> + * Disable the transmitter. >> + * This is mandatory when DMA is used, otherwise the DMA buffer >> + * is fully transmitted. >> + */ >> + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS); >> + >> /* Disable interrupts */ >> atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask); >> >> @@ -513,6 +521,9 @@ static void atmel_start_tx(struct uart_port *port) >> >> /* Enable interrupts */ >> atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask); >> + >> + /* re-enable the transmitter */ >> + atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN); >> } >> >> /* > > Hi Richard, Hi ! > > I've just discovered that I have some weird behavior with this patch. On > current Linus' tree, with sama5d2 + DMA, I see some garbage characters > coming out of the console when I try to stop my system (reboot/halt) [1]. Yes, I've also seen that on my board, it was on my todo list. (Although I didn't know this patch was the culprit !) > > Moreover, and I do understand that it's not the problem right here, when > applied on our linux-4.4-at91 branch (our vendor tree actually), it > hangs the boot process as it seems that a burst of open/close of the > serial port happens while starting the rootfs. It's definitively my own > problem, but it can bring light to what we are seeing on Mainline... aïe ! This is clearly something we'll have to understand ! > I think that we may need to flush the DMA channel in this > atmel_stop_tx() function. > yes, I'll look into that. > Best regards, > > [1] If you want to test, you need to apply this patch for eMMC BTW: > https://patchwork.kernel.org/patch/9617489/ > > -- > Nicolas Ferre Thanks ! Richard.
Re: [PATCH 2/2] drm/panel: simple: Add support for Winstar WF35LTIACD
On 12/03/2017 15:17, Rob Herring wrote: > On Fri, Mar 03, 2017 at 04:21:56PM +0100, Richard Genoud wrote: >> This adds support for the Winstar Display Co. WF35LTIACD 3.5" QVGA TFT >> LCD panel, which can be supported by the simple panel driver. >> >> Signed-off-by: Richard Genoud >> --- >> .../bindings/display/panel/winstar,wf35ltiacd | 7 ++ >> drivers/gpu/drm/panel/panel-simple.c | 28 >> ++ >> 2 files changed, 35 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd >> >> diff --git >> a/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd >> b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd >> new file mode 100644 >> index ..68408adf239f >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd >> @@ -0,0 +1,7 @@ >> +Winstar Display Corporation 3.5" QVGA (320x240) TFT LCD panel >> + >> +Required properties: >> +- compatible: should be "winstar,wf35ltiacd" >> + >> +This binding is compatible with the simple-panel binding, which is specified >> +in simple-panel.txt in this directory. > > Please specify the power supply(ies). I know a lot of bindings haven't, > but many panels have more than 1 supply and just relying on simple panel > doesn't distinguish whether the panel has 1 supply or you didn't check > and it has more than 1. > > Rob > Ok, I'll specify the power supply. And I'll add an example, it's always nice to have one. Thanks. Richard.
Re: [REGRESSION] Two issues that prevent process accounting (taskstats) from working correctly
Cc += Al Viro
2017-02-12 16:44 GMT+01:00 Dmitry Romanov :
> On Mon, Dec 19, 2016 at 01:06:00PM +0100, Martin Steigerwald wrote:
>>
>> 1) Sometimes process accounting does not work at all.
>>
>> The acct() system call (to activate process accounting) return value 0,
>> which means that process accounting is activated successfully.
>> However, no process accounting records are written whatsoever. This
>> situation can be reproduced with the program 'acctdemo.c'
>> that you can find as attachment. When this program gives the message
>> "found a process accounting record!", the situation is okay
>> and process accounting works fine to the file '/tmp/mypacct'. When the
>> message 'No process accounting record yet' is repeatedly given,
>> process accounting does not work and will not work at all. It might be
>> that you have to start this program several times before you get
>> this situation (preferably start/finish lots of processes in the mean time).
>> This problem is probably caused by a new mechanism introduced in the
>> kernel code (/linux/kernel/acct.c) that is called 'slow accounting'
>> and has to be solved in the kernel code.
>>
>> I experience this problem on Debian8 with a 4.8 kernel and on CentOS7
>> with a 4.8 kernel.
>
> I reported same problem on bugzilla as:
>
> Bug 180841 - Process accounting sometimes do not append records for terminated
> processes
> https://bugzilla.kernel.org/show_bug.cgi?id=180841
>
> I think I found cause of this problem and can suggest patch which correct this
> problem.
>
> Problem arise in such situation:
>
> Problem arise if between process accounting activation with call acct(...) and
> first termination of process pass not less than one jiffy. (Note, acct()
> return
> successfully, with zero.) In such situation records for terminated processes
> do nod append to accounting file, until process accounting is restarted.
>
> I wrote test program test.c for illustration described problem for kernel
> version 3.17-rc1 and later. This program create empty file for accounting,
> call system call acct() with this file, sleep for not less than one jiffy,
> create new process and exit this process. Then, program test size of
> accounting
> file. If size of file remain zero, it seems problem and program display
> message
> "Accounting file size = 0, process accounting did not add record to accounting
> file!".
> On my system problem reproduce almost always by this test.c.
>
> --
> test.c
> --
> #include
> #include
> #include
> #include
> #include
> #include
>
> /* Accounting file name : */
> #define ACCTFILENAME "/var/log/pacct"
>
> int main()
> {
> int fd;
> int pid;
> struct stat sb;
>
> /* Turn off accounting */
> if ( acct(NULL) < 0 ) {
> perror("Process accounting off");
> return -1;
> }
>
> /* Create empty accounting file */
> fd = creat(ACCTFILENAME, S_IRUSR | S_IWUSR);
> if ( fd == -1 ) {
> perror("Accounting file creation");
> return -1;
> }
> if ( close(fd) == -1) {
> perror("Accounting file closing");
> return -1;
> }
>
> /* Switch accounting on */
> if ( acct(ACCTFILENAME) < 0 ) {
> perror("Process accounting on");
> return -1;
> }
>
> /* Delay must be at least 1 jiffy.
> For reproducing bug, some process exit must not happen during first
> jiffy.
> If HZ >= 100, need delay minimum 10 ms. */
> usleep(1);
>
> /* Create and exit child process. The record for this process must be
> append
> by activated process accounting. */
> pid = fork();
> if (pid < 0) {
> perror("Child process creating");
> return -1;
> }
> /* Exit child process */
> if (pid == 0) {
> exit(0);
> }
> /* Wait for child process termination */
> wait(NULL);
>
> /* Get size of accounting file. */
> if ( stat(ACCTFILENAME, &sb) == -1 ) {
> perror("Getting accounting file status");
> return -1;
> }
>
> if ( sb.st_size == 0 ) {
> printf("Accounting file size = 0, process accounting did not
> add record to accounting file!\n");
> }
> else {
> printf("Accounting file size > 0, expected behaviour.\n");
> }
>
> /* Turn off accounting and remove accounting file*/
> if ( acct(NULL) < 0 ) {
> perror("Process accounting off");
> return -1;
> }
> if ( remove(ACCTFILENAME) == -1 ) {
> perror("Removing accounting file");
> return -1;
> }
>
> return 0;
>
> }
> --
>
> I suppose that this problem may be solve in kernel versions 3.17-rc1 and
[PATCH 1/2] dt-bindings: mfd: Fix path to atmel/hlcdc-dc.txt
Atmel hlcdc-dc documentation is actually in Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt Signed-off-by: Richard Genoud --- Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt index 670831b29565..eec40be7f79a 100644 --- a/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt +++ b/Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt @@ -15,7 +15,7 @@ Required properties: The HLCDC IP exposes two subdevices: - a PWM chip: see ../pwm/atmel-hlcdc-pwm.txt - - a Display Controller: see ../display/atmel-hlcdc-dc.txt + - a Display Controller: see ../display/atmel/hlcdc-dc.txt Example:
[PATCH 2/2] dt-bindings: display: atmel: Fix path to atmel-hlcdc.txt
The relative path pointed to Documentation/devicetree/bindings/display/mfd/atmel-hlcdc.txt instead of Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt Signed-off-by: Richard Genoud --- Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt index ebc1a914bda3..ec94468b35be 100644 --- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt +++ b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt @@ -1,7 +1,7 @@ Device-Tree bindings for Atmel's HLCDC (High LCD Controller) DRM driver The Atmel HLCDC Display Controller is subdevice of the HLCDC MFD device. -See ../mfd/atmel-hlcdc.txt for more details. +See ../../mfd/atmel-hlcdc.txt for more details. Required properties: - compatible: value should be "atmel,hlcdc-display-controller"
[PATCH 2/2] drm/panel: simple: Add support for Winstar WF35LTIACD
This adds support for the Winstar Display Co. WF35LTIACD 3.5" QVGA TFT
LCD panel, which can be supported by the simple panel driver.
Signed-off-by: Richard Genoud
---
.../bindings/display/panel/winstar,wf35ltiacd | 7 ++
drivers/gpu/drm/panel/panel-simple.c | 28 ++
2 files changed, 35 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
diff --git a/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
new file mode 100644
index ..68408adf239f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/winstar,wf35ltiacd
@@ -0,0 +1,7 @@
+Winstar Display Corporation 3.5" QVGA (320x240) TFT LCD panel
+
+Required properties:
+- compatible: should be "winstar,wf35ltiacd"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 06aaf79de8c8..5d45664c6c0e 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1713,6 +1713,31 @@ static const struct panel_desc urt_umsh_8596md_parallel
= {
.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
};
+static const struct drm_display_mode winstar_wf35ltiacd_mode = {
+ .clock = 6410,
+ .hdisplay = 320,
+ .hsync_start = 320 + 20,
+ .hsync_end = 320 + 20 + 30,
+ .htotal = 320 + 20 + 30 + 38,
+ .vdisplay = 240,
+ .vsync_start = 240 + 4,
+ .vsync_end = 240 + 4 + 3,
+ .vtotal = 240 + 4 + 3 + 15,
+ .vrefresh = 60,
+ .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static const struct panel_desc winstar_wf35ltiacd = {
+ .modes = &winstar_wf35ltiacd_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 70,
+ .height = 53,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+};
+
static const struct of_device_id platform_of_match[] = {
{
.compatible = "ampire,am800480r3tmqwa1h",
@@ -1892,6 +1917,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "urt,umsh-8596md-20t",
.data = &urt_umsh_8596md_parallel,
}, {
+ .compatible = "winstar,wf35ltiacd",
+ .data = &winstar_wf35ltiacd,
+ }, {
/* sentinel */
}
};
[PATCH 1/2] devicetree: add vendor prefix for Winstar Display Corp.
Winstar Display Corp. is specialized in LCD displays for embedded products. cf: http://www.winstar.com.tw Signed-off-by: Richard Genoud --- Documentation/devicetree/bindings/vendor-prefixes.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index 16d3b5e7f5d1..56df3afd625a 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -322,6 +322,7 @@ voipac Voipac Technologies s.r.o. wd Western Digital Corp. wexler Wexler winbond Winbond Electronics corp. +winstarWinstar Display Corp. wlfWolfson Microelectronics wm Wondermedia Technologies, Inc. x-powers X-Powers
[PATCH 1/4] tty/serial: atmel: move atmel_serial header into driver directory
atmel_serial.h is only used by atmel_serial.c, so there's no need for
it to lie in include/linux.
Suggested-by: Joe Perches
Signed-off-by: Richard Genoud
---
MAINTAINERS | 2 +-
drivers/tty/serial/atmel_serial.c| 2 +-
{include/linux => drivers/tty/serial}/atmel_serial.h | 0
3 files changed, 2 insertions(+), 2 deletions(-)
rename {include/linux => drivers/tty/serial}/atmel_serial.h (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index ad4f2ce991ca..d7b07cb23cbe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8218,7 +8218,7 @@ MICROCHIP / ATMEL AT91 / AT32 SERIAL DRIVER
M: Richard Genoud
S: Maintained
F: drivers/tty/serial/atmel_serial.c
-F: include/linux/atmel_serial.h
+F: drivers/tty/serial/atmel_serial.h
MICROCHIP / ATMEL DMA DRIVER
M: Ludovic Desroches
diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index dcebb28ffbc4..ad6c2bdea481 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -38,7 +38,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -71,6 +70,7 @@
#include
#include "serial_mctrl_gpio.h"
+#include "atmel_serial.h"
static void atmel_start_rx(struct uart_port *port);
static void atmel_stop_rx(struct uart_port *port);
diff --git a/include/linux/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
similarity index 100%
rename from include/linux/atmel_serial.h
rename to drivers/tty/serial/atmel_serial.h
[PATCH 4/4] tty/serial: sh-sci: remove uneeded IS_ERR_OR_NULL calls
Since commit 1d267ea6539f ("serial: mctrl-gpio: simplify init routine"),
the mctrl_gpio_to_gpiod() function can't return an error anymore.
So, just testing for a NULL pointer is ok.
Signed-off-by: Richard Genoud
---
drivers/tty/serial/sh-sci.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 9a47cc4f16a2..36fde6f24040 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1934,12 +1934,12 @@ static unsigned int sci_get_mctrl(struct uart_port
*port)
if (s->autorts) {
if (sci_get_cts(port))
mctrl |= TIOCM_CTS;
- } else if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS))) {
+ } else if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_CTS)) {
mctrl |= TIOCM_CTS;
}
- if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DSR)))
+ if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_DSR))
mctrl |= TIOCM_DSR;
- if (IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(gpios, UART_GPIO_DCD)))
+ if (!mctrl_gpio_to_gpiod(gpios, UART_GPIO_DCD))
mctrl |= TIOCM_CAR;
return mctrl;
@@ -3072,10 +3072,8 @@ static int sci_probe_single(struct platform_device *dev,
return PTR_ERR(sciport->gpios);
if (sciport->has_rtscts) {
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(sciport->gpios,
- UART_GPIO_CTS)) ||
- !IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(sciport->gpios,
- UART_GPIO_RTS))) {
+ if (mctrl_gpio_to_gpiod(sciport->gpios, UART_GPIO_CTS) ||
+ mctrl_gpio_to_gpiod(sciport->gpios, UART_GPIO_RTS)) {
dev_err(&dev->dev, "Conflicting RTS/CTS config\n");
return -EINVAL;
}
[PATCH 3/4] tty/serial: mxs-auart: remove uneeded IS_ERR_OR_NULL calls
Since commit 1d267ea6539f ("serial: mctrl-gpio: simplify init routine"),
the mctrl_gpio_to_gpiod() function can't return an error anymore.
So, just testing for a NULL pointer is ok.
Signed-off-by: Richard Genoud
---
drivers/tty/serial/mxs-auart.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 6989b227d134..541933d8a5c6 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -973,10 +973,8 @@ static int mxs_auart_dma_init(struct mxs_auart_port *s)
}
-#define RTS_AT_AUART() IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,\
- UART_GPIO_RTS))
-#define CTS_AT_AUART() IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(s->gpios,\
- UART_GPIO_CTS))
+#define RTS_AT_AUART() (mctrl_gpio_to_gpiod(s->gpios, UART_GPIO_RTS) == NULL)
+#define CTS_AT_AUART() (mctrl_gpio_to_gpiod(s->gpios, UART_GPIO_CTS) == NULL)
static void mxs_auart_settermios(struct uart_port *u,
struct ktermios *termios,
struct ktermios *old)
[PATCH 2/4] tty/serial: atmel: remove uneeded IS_ERR_OR_NULL calls
Since commit 1d267ea6539f ("serial: mctrl-gpio: simplify init routine"),
the mctrl_gpio_to_gpiod() function can't return an error anymore.
So, just testing for a NULL pointer is ok.
Signed-off-by: Richard Genoud
---
drivers/tty/serial/atmel_serial.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index ad6c2bdea481..e0f1955b3c7e 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -315,32 +315,28 @@ static unsigned int atmel_get_lines_status(struct
uart_port *port)
mctrl_gpio_get(atmel_port->gpios, &ret);
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_CTS))) {
+ if (mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_CTS)) {
if (ret & TIOCM_CTS)
status &= ~ATMEL_US_CTS;
else
status |= ATMEL_US_CTS;
}
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_DSR))) {
+ if (mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_DSR)) {
if (ret & TIOCM_DSR)
status &= ~ATMEL_US_DSR;
else
status |= ATMEL_US_DSR;
}
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_RI))) {
+ if (mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_RI)) {
if (ret & TIOCM_RI)
status &= ~ATMEL_US_RI;
else
status |= ATMEL_US_RI;
}
- if (!IS_ERR_OR_NULL(mctrl_gpio_to_gpiod(atmel_port->gpios,
- UART_GPIO_DCD))) {
+ if (mctrl_gpio_to_gpiod(atmel_port->gpios, UART_GPIO_DCD)) {
if (ret & TIOCM_CD)
status &= ~ATMEL_US_DCD;
else
Re: [PATCH] tty/serial: atmel: remove cache when unnecessary
2017-02-21 13:03 GMT+01:00 Alexandre Belloni
:
> struct cache is only used in suspend/resume. Exclude it when PM is not
> selected.
>
> Suggested-by: Richard Genoud
> Signed-off-by: Alexandre Belloni
Acked-by: Richard Genoud
> ---
> drivers/tty/serial/atmel_serial.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index dcebb28ffbc4..f0d4894267c2 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -175,6 +175,7 @@ struct atmel_uart_port {
> unsigned intpending_status;
> spinlock_t lock_suspended;
>
> +#ifdef CONFIG_PM
> struct {
> u32 cr;
> u32 mr;
> @@ -185,6 +186,7 @@ struct atmel_uart_port {
> u32 fmr;
> u32 fimr;
> } cache;
> +#endif
>
> int (*prepare_rx)(struct uart_port *port);
> int (*prepare_tx)(struct uart_port *port);
> --
> 2.11.0
>
Thanks !
Re: [PATCH] tty/serial: atmel: increase ATMEL_MAX_UART
2017-02-21 13:03 GMT+01:00 Alexandre Belloni
:
> The samx7 family uses the same UART/USART IP as the at91/sama5 families but
> has 8 of those.
>
> Suggested-by: Szemző András
> Signed-off-by: Alexandre Belloni
Acked-by: Richard Genoud
> ---
> drivers/tty/serial/atmel_serial.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index f0d4894267c2..8cc152e67bfb 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -119,8 +119,9 @@ struct atmel_uart_char {
> /*
> * at91: 6 USARTs and one DBGU port (SAM9260)
> * avr32: 4
> + * samx7: 3 USARTs and 5 UARTs
> */
> -#define ATMEL_MAX_UART 7
> +#define ATMEL_MAX_UART 8
>
> /*
> * We wrap our port structure around the generic uart_port.
> --
> 2.11.0
>
Thanks !
Re: [PATCH] atmel_serial: Use the fractional divider when possible
2017-02-10 16:49 GMT+01:00 Ludovic Desroches :
> On Fri, Feb 10, 2017 at 04:24:46PM +0100, Romain Izard wrote:
>> The fractional baud rate generator is available when using the
>> asynchronous mode of Atmel USART controllers. It makes it possible to
>> use higher baudrates, in exchange for a less precise clock with a
>> variable duty cycle.
>>
>> The existing code restricts its use to the normal mode of the USART
>> controller, following the recommendation from the datasheet for the
>> first chip embedding this type of controller. This recommendation has
>> been removed from the documentation for the newer chips. After
>> verification, all revisions of this controller should be able to use the
>> fractional baud rate generator with the different asynchronous modes.
>>
>> Removing the condition on ATMEL_US_USMODE makes it possible to get
>> correct baudrates at high speed in more cases.
>>
>> This was tested with a board using an Atmel SAMA5D2 chip and a TI
>> WL1831 WiFi/Bluetooth combo chip at 3 Mbauds, with hardware flow control
>> enabled.
>>
>> Signed-off-by: Romain Izard
>
> Acked-by: Ludovic Desroches
Signed-off-by: Richard Genoud
Thanks !
> Thanks
>
>> ---
>> drivers/tty/serial/atmel_serial.c | 7 ---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/tty/serial/atmel_serial.c
>> b/drivers/tty/serial/atmel_serial.c
>> index fabbe76203bb..6684456dca9e 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -1758,7 +1758,9 @@ static void atmel_get_ip_name(struct uart_port *port)
>>
>> /*
>>* Only USART devices from at91sam9260 SOC implement fractional
>> - * baudrate.
>> + * baudrate. It is available for all asynchronous modes, with the
>> + * following restriction: the sampling clock's duty cycle is not
>> + * constant.
>>*/
>> atmel_port->has_frac_baudrate = false;
>> atmel_port->has_hw_timer = false;
>> @@ -2202,8 +2204,7 @@ static void atmel_set_termios(struct uart_port *port,
>> struct ktermios *termios,
>>* then
>>* 8 CD + FP = selected clock / (2 * baudrate)
>>*/
>> - if (atmel_port->has_frac_baudrate &&
>> - (mode & ATMEL_US_USMODE) == ATMEL_US_USMODE_NORMAL) {
>> + if (atmel_port->has_frac_baudrate) {
>> div = DIV_ROUND_CLOSEST(port->uartclk, baud * 2);
>> cd = div >> 3;
>> fp = div & ATMEL_US_FP_MASK;
>> --
>> 2.9.3
>>
