From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AC9143CB2C9 for ; Mon, 30 Mar 2026 12:08:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774872496; cv=none; b=Do7XLOxTx8n+d2M4q2ELyF2zx9uZs49xmncugyM4bjkgZQjr70HBh32xlWibZ74bIux+fU/Iu9IktXDOF4a7XZkdtSRBWos3vDyRdAzhEt3+itC9od/jsQunQvYd4TnQbR7rX0wdmHXvTWg3ZU0GTtjE07ffXDZPsbfImtloNh8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774872496; c=relaxed/simple; bh=OIy0Ykp4CDvpi6WM0Dj8UPcoA11Hbs673I/Y6Z+ZfsY=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=u/lE/cKjzfr86lqrE2qM2Q1wexDkYKNilUn8M2EIzsELdLlGSjt2r7jKjEneP6YIr/o0Xi/X521TAZ/1TKSXDZQoBEMxwJ7cQgpJ92u0JGoNrxeFYnuG5hQ8t1Yrohqeqjy7GqXgQBf7j0lzEMpEZ3EuQQikzz1lz+cAaHvy7Ic= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Vw+/EpaR; arc=none smtp.client-ip=209.85.210.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Vw+/EpaR" Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-82735a41920so1427197b3a.2 for ; Mon, 30 Mar 2026 05:08:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1774872494; x=1775477294; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=uL7Sv/Z7y7JRc1HGEqoBM0qw9rixKMSWYx/opbAOEeQ=; b=Vw+/EpaR1v+t0NPLpkswswAOHnOdZgHRhkiIzRP+1VA4J/m0G5dVjk/vp50Mf+amKv 6ZAbMPt8eSqbdtKCbv8cfOdbpQmaDTRvcDwX2b69f1Mg/SJyz8c7Zu8fyv1e6bHClftz +zGL3drrjRwaqz1K8WjnW1Zi7BLT9BFzr5tUx/lwx20XFbbYhteGZPNQE1j+hsPgUYrU DYW0YlpKUFxp47Zr7EwVUCjtAZPJC/sbWWJev0lJrUbdzqWZGEPc1KMmog9LjVY+tztl iZBLgEiVEHKyBy6yL5gLQLBrvn1DVN8ChRUUKsl4pepJ0WpZ3RurjfWCUxmDhM666Mau cN1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774872494; x=1775477294; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=uL7Sv/Z7y7JRc1HGEqoBM0qw9rixKMSWYx/opbAOEeQ=; b=kjKQ+PrF046t9oJ0eCipWR97HnzKLjixY3U0OjAm8kgl348QfRqT8YWTAAlZZf3UpO 7tkeAq6LO1Lcg4NFVWJ057CDQJ12ilqTpDMCVdMjL5299rH9+V7gaV+0u6u1lu1JC7me CsY/UuoZk0hl4A1TGfFcyW0uyY10ENOfs3/vt8rC+/q+BsI8IggcRTRhJYRYUFEp3YZt zuLBLrVxp9tTEXuVtrAv9GrFVjIqMGxid6hGoCs1UeXZLynvGdiVdK4MVBA5qGfN5/6j UWS+Wk2FNsoGZXwX4R6sm+e6/PI5YxI+bXgrbjP47xXre/HLekTg2QhQybIGSEVUll/z UeaQ== X-Forwarded-Encrypted: i=1; AJvYcCUF1htZWGd7c6vS1ZVxZefQNXJb4bGWyHzVW4R6EG2T7r3glmg0U1j/syeZstuP893foOtCrArjJa69dYI=@vger.kernel.org X-Gm-Message-State: AOJu0Yy4hQbh3jsTQdV5hZ56Vn9bXVDtw0R4OVgN57mW1Tk2mhr5CqFo KBu+CBU31Y3/eQo6uo7JubhTS6ju7gQyq46uqKKkjtzEA1cWE5u2+Ajb X-Gm-Gg: ATEYQzw+yiVswG9Yvfir4U13LqJ9/03BMrjBWnLzCUGC0MowA78GNhAqc6Je6Kg9FKn OSRvnwcGdrTU8Iel8faehShf4ytY6m7nuceFDiuLmlAUJS/OdOAUM3aDKijeS9dFbYBhB684vSF jlU/7Z3MBxeX+M+E0M+QIGYMOT2mP9V6YpB1ljewyRrMyqHpsFhjnQo6JqqilkmWuQYR/xkZgH1 v+BWBnLiY9sL5EcNllQ2WFKCZILdrFZALiMZE3soVY3MpNGvA89j16XKLFnVMaCLWA9jjOgE5qy 1nxnRHlTQMvOtedZIs3kS1ImPvBp1pC0zD/1VlfXbVUNzfqLfRl1pbNtZ+fzylUSOHIz8rrIkyc oOQX2JSHi21EGrbV5+ihRVBAz6S1s5rBCmXL/IJUqzLGoL3ot1hYrN2RQMkj3OffB0odq9NeLsv qw792/MHLkGgs301UkdVtDCalJdq+9OTva1Xd1TONg6BOGfAS1qs5F5Pzo/5LxmRUwxv3F0+Hwo KRHCgEjZ3Jt7Fqt4InuS6YdeU1igomoDoz2UDRd8jXgNhL8EUdCEmepUX0M X-Received: by 2002:a05:6a00:140d:b0:82c:6b23:6d10 with SMTP id d2e1a72fcca58-82c95d248d4mr11901289b3a.3.1774872493998; Mon, 30 Mar 2026 05:08:13 -0700 (PDT) Received: from c8971f1abf06.ap-southeast-2.compute.internal (ec2-3-106-126-184.ap-southeast-2.compute.amazonaws.com. [3.106.126.184]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-82ca860b125sm7137780b3a.50.2026.03.30.05.08.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Mar 2026 05:08:12 -0700 (PDT) From: Weigang He To: greybus-dev@lists.linaro.org, linux-kernel@vger.kernel.org Cc: Weigang He , Ayush Singh , Johan Hovold , Alex Elder , Greg Kroah-Hartman Subject: [PATCH 2/2] greybus: gb-beagleplay: propagate hdlc_tx_frames() errors to callers Date: Mon, 30 Mar 2026 12:08:01 +0000 Message-Id: <20260330120801.981506-2-geoffreyhe2@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20260330120801.981506-1-geoffreyhe2@gmail.com> References: <20260330120801.981506-1-geoffreyhe2@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that hdlc_tx_frames() can drop frames when the circular buffer is full, make the failure visible to callers: - Change hdlc_tx_frames() return type from void to int (-EAGAIN on buffer full). - Change gb_beagleplay_start_svc() / gb_beagleplay_stop_svc() to return int so probe and firmware-upload paths can detect failures. - gb_message_send(): propagate the error so the greybus core can handle the transport failure. - hdlc_tx_s_frame_ack(): log with dev_warn_ratelimited on failure (ACK loss is recoverable by HDLC retransmission). - Probe path: propagate start_svc failure via new free_greybus label. - Firmware upload paths: return FW_UPLOAD_ERR_RW_ERROR when SVC restart fails instead of silently continuing. - Remove path: best-effort stop_svc, ignore failure. Cc: Ayush Singh Cc: Johan Hovold Cc: Alex Elder Cc: Greg Kroah-Hartman Signed-off-by: Weigang He --- drivers/greybus/gb-beagleplay.c | 64 ++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index da1b9039fd2f3..55b3ad4b3c360 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -314,6 +314,9 @@ static void hdlc_transmit(struct work_struct *work) * @payloads: array of payload buffers * @count: number of payloads * + * Every data byte may need HDLC escaping (doubling its size). + * Frame layout: flag(1) + address(1-2) + control(1-2) + payload + CRC(2-4) + flag(1). + * * Returns the maximum number of bytes needed in the circular buffer. */ static size_t hdlc_encoded_length(const struct hdlc_payload payloads[], @@ -348,8 +351,10 @@ static size_t hdlc_encoded_length(const struct hdlc_payload payloads[], * available, then verifies space under the lock and writes the entire * frame atomically. Either a complete frame is enqueued or nothing is * written, avoiding both sleeping in atomic context and partial frames. + * + * Returns 0 on success, -EAGAIN if the buffer remains full after retries. */ -static void hdlc_tx_frames(struct gb_beagleplay *bg, u8 address, u8 control, +static int hdlc_tx_frames(struct gb_beagleplay *bg, u8 address, u8 control, const struct hdlc_payload payloads[], size_t count) { size_t needed = hdlc_encoded_length(payloads, count); @@ -372,25 +377,24 @@ static void hdlc_tx_frames(struct gb_beagleplay *bg, u8 address, u8 control, } if (retries < 0) { - dev_warn_ratelimited(&bg->sd->dev, - "Tx circ buf full, dropping frame\n"); - return; + dev_warn_ratelimited(&bg->sd->dev, "Tx circ buf full, dropping frame\n"); + return -EAGAIN; } spin_lock(&bg->tx_producer_lock); /* - * Re-check under the lock. Should not fail since - * tx_producer_lock serialises all producers and the - * consumer only frees space, but guard against it. + * Re-check space under the lock to close the TOCTOU window. + * This should be rare since tx_producer_lock serialises all + * producers and the consumer only frees space. If it fires, + * the caller is expected to handle -EAGAIN (retry or report). */ head = bg->tx_circ_buf.head; tail = READ_ONCE(bg->tx_circ_buf.tail); if (unlikely(CIRC_SPACE(head, tail, TX_CIRC_BUF_SIZE) < needed)) { spin_unlock(&bg->tx_producer_lock); - dev_warn_ratelimited(&bg->sd->dev, - "Tx circ buf space lost, dropping frame\n"); - return; + dev_warn_ratelimited(&bg->sd->dev, "Tx circ buf space lost, dropping frame\n"); + return -EAGAIN; } hdlc_append_tx_frame(bg); @@ -406,11 +410,16 @@ static void hdlc_tx_frames(struct gb_beagleplay *bg, u8 address, u8 control, spin_unlock(&bg->tx_producer_lock); schedule_work(&bg->tx_work); + return 0; } static void hdlc_tx_s_frame_ack(struct gb_beagleplay *bg) { - hdlc_tx_frames(bg, bg->rx_buffer[0], (bg->rx_buffer[1] >> 1) & 0x7, NULL, 0); + int ret; + + ret = hdlc_tx_frames(bg, bg->rx_buffer[0], (bg->rx_buffer[1] >> 1) & 0x7, NULL, 0); + if (ret) + dev_warn_ratelimited(&bg->sd->dev, "Failed to send HDLC ACK: %d\n", ret); } static void hdlc_rx_frame(struct gb_beagleplay *bg) @@ -668,6 +677,7 @@ static int gb_message_send(struct gb_host_device *hd, u16 cport, struct gb_messa struct gb_beagleplay *bg = dev_get_drvdata(&hd->dev); struct hdlc_payload payloads[3]; __le16 cport_id = cpu_to_le16(cport); + int ret; dev_dbg(&hd->dev, "Sending greybus message with Operation %u, Type: %X on Cport %u", msg->header->operation_id, msg->header->type, cport); @@ -682,7 +692,10 @@ static int gb_message_send(struct gb_host_device *hd, u16 cport, struct gb_messa payloads[2].buf = msg->payload; payloads[2].len = msg->payload_size; - hdlc_tx_frames(bg, ADDRESS_GREYBUS, 0x03, payloads, 3); + ret = hdlc_tx_frames(bg, ADDRESS_GREYBUS, 0x03, payloads, 3); + if (ret) + return ret; + greybus_message_sent(bg->gb_hd, msg, 0); return 0; @@ -695,20 +708,20 @@ static void gb_message_cancel(struct gb_message *message) static struct gb_hd_driver gb_hdlc_driver = { .message_send = gb_message_send, .message_cancel = gb_message_cancel }; -static void gb_beagleplay_start_svc(struct gb_beagleplay *bg) +static int gb_beagleplay_start_svc(struct gb_beagleplay *bg) { const u8 command = CONTROL_SVC_START; const struct hdlc_payload payload = { .len = 1, .buf = (void *)&command }; - hdlc_tx_frames(bg, ADDRESS_CONTROL, 0x03, &payload, 1); + return hdlc_tx_frames(bg, ADDRESS_CONTROL, 0x03, &payload, 1); } -static void gb_beagleplay_stop_svc(struct gb_beagleplay *bg) +static int gb_beagleplay_stop_svc(struct gb_beagleplay *bg) { const u8 command = CONTROL_SVC_STOP; const struct hdlc_payload payload = { .len = 1, .buf = (void *)&command }; - hdlc_tx_frames(bg, ADDRESS_CONTROL, 0x03, &payload, 1); + return hdlc_tx_frames(bg, ADDRESS_CONTROL, 0x03, &payload, 1); } static int cc1352_bootloader_wait_for_ack(struct gb_beagleplay *bg) @@ -946,7 +959,9 @@ static enum fw_upload_err cc1352_prepare(struct fw_upload *fw_upload, gb_greybus_deinit(bg); msleep(5 * MSEC_PER_SEC); - gb_beagleplay_stop_svc(bg); + /* Best effort — device is entering bootloader mode regardless. */ + if (gb_beagleplay_stop_svc(bg)) + dev_warn(&bg->sd->dev, "Failed to send SVC stop before flashing\n"); msleep(200); flush_work(&bg->tx_work); @@ -988,7 +1003,9 @@ static enum fw_upload_err cc1352_prepare(struct fw_upload *fw_upload, if (gb_greybus_init(bg) < 0) return dev_err_probe(&bg->sd->dev, FW_UPLOAD_ERR_RW_ERROR, "Failed to initialize greybus"); - gb_beagleplay_start_svc(bg); + if (gb_beagleplay_start_svc(bg)) + return dev_err_probe(&bg->sd->dev, FW_UPLOAD_ERR_RW_ERROR, + "Failed to restart SVC after skip"); return FW_UPLOAD_ERR_FW_INVALID; } @@ -1069,7 +1086,9 @@ static enum fw_upload_err cc1352_poll_complete(struct fw_upload *fw_upload) return dev_err_probe(&bg->sd->dev, FW_UPLOAD_ERR_RW_ERROR, "Failed to initialize greybus"); - gb_beagleplay_start_svc(bg); + if (gb_beagleplay_start_svc(bg) < 0) + return dev_err_probe(&bg->sd->dev, FW_UPLOAD_ERR_RW_ERROR, + "Failed to start SVC"); return FW_UPLOAD_ERR_NONE; } @@ -1180,10 +1199,14 @@ static int gb_beagleplay_probe(struct serdev_device *serdev) if (ret) goto free_fw; - gb_beagleplay_start_svc(bg); + ret = gb_beagleplay_start_svc(bg); + if (ret) + goto free_greybus; return 0; +free_greybus: + gb_greybus_deinit(bg); free_fw: gb_fw_deinit(bg); free_hdlc: @@ -1199,6 +1222,7 @@ static void gb_beagleplay_remove(struct serdev_device *serdev) gb_fw_deinit(bg); gb_greybus_deinit(bg); + /* Best effort — device is being removed. */ gb_beagleplay_stop_svc(bg); hdlc_deinit(bg); gb_serdev_deinit(bg); -- 2.34.1