Message ID | 20231207-tps6598x_update-v1-2-dc21b5301d91@wolfvision.net |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp4723374vqy; Thu, 7 Dec 2023 03:51:39 -0800 (PST) X-Google-Smtp-Source: AGHT+IHF3bsIQdGpe6j/vZJeXQBSTdrSkMW1nn5DLAIwgVPCtBsvzuD78UkSQuOuhTpVq2H/29Jw X-Received: by 2002:a17:90b:3c11:b0:288:9282:7c1a with SMTP id pb17-20020a17090b3c1100b0028892827c1amr940604pjb.55.1701949898760; Thu, 07 Dec 2023 03:51:38 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1701949898; cv=pass; d=google.com; s=arc-20160816; b=xt8T7IlbMrWSCpDFpBVVCOKzFmFROPZbx325G9kmTyKGHa7CUynswQbR3URuwscRJu Y9kKFFHWKV44k9Thncyc5l2LzmKTdtWnpby5s7gyiincoqxOdTj5cPZ3oEOm4OALctMe lc1cmVN/V8OCLK36olzxUE925QEZ8ErEVcH7zLt1lHNT+DjHuScbbISONB96PwF1HJ4r E/VswUPECc0yLQDIsyOABVYxfpy+rQrNfeAcfVpdRtt2X+wJfT32IhXGl+8PypC7h4o7 WKzahgrUnD/rsRSiUqVmW6/iu9JZdce+oze4LmIbJrdo1WZQlpIn3IOxHRKc6ZHWg76W +pRg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:cc:to:in-reply-to:references :message-id:content-transfer-encoding:subject:date:from :dkim-signature; bh=XOVUwsCdesOAxIwKuBpk7IheoIyYIMKYs3+/KWhnNw8=; fh=skg4aWcIne/NkBvGBSxiPFt6u1LkY77F5jj9Umi9ZmE=; b=WSo+jzqxt4rau6FVRP0oXHF9I1ZSKW5dik0XTsSN3RH6D5CpCroOL4QrG1JdE6XaFv VPMMDCvDyyHkBlT5Tu+LuULXWsjhiOpMMxsZ8WHvcAciut8+tZxVrCaq158V8ZYpZGbW porQuOqAhGEvGll8vS++3V6lMEcDakSZK+xZDKE9B8uuj94/7x+PAAGld+Q3MJmNmuQ4 bao2tAEMwdsW+7EN8k5OtQXWBkE2NaB1G8ieki/0JelGUokaGmf7XwXz+xcqvIdPnoiV /mFITUVMBb1RgE2akPOTbviRPdMLLHLuz/k1LMhNAtZpOkOA/EBgG/ysFlq2KDtsVKfP gOPA== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@wolfvision.net header.s=selector2 header.b=yT73oPMJ; arc=pass (i=1 spf=pass spfdomain=wolfvision.net dkim=pass dkdomain=wolfvision.net dmarc=pass fromdomain=wolfvision.net); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=wolfvision.net Received: from lipwig.vger.email (lipwig.vger.email. [23.128.96.33]) by mx.google.com with ESMTPS id ob4-20020a17090b390400b0028868509509si979099pjb.108.2023.12.07.03.51.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 03:51:38 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33; Authentication-Results: mx.google.com; dkim=pass header.i=@wolfvision.net header.s=selector2 header.b=yT73oPMJ; arc=pass (i=1 spf=pass spfdomain=wolfvision.net dkim=pass dkdomain=wolfvision.net dmarc=pass fromdomain=wolfvision.net); spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=wolfvision.net Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by lipwig.vger.email (Postfix) with ESMTP id 028EE807C763; Thu, 7 Dec 2023 03:51:36 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at lipwig.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1379083AbjLGLvT (ORCPT <rfc822;pusanteemu@gmail.com> + 99 others); Thu, 7 Dec 2023 06:51:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49874 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232435AbjLGLvO (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 7 Dec 2023 06:51:14 -0500 Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05on2064.outbound.protection.outlook.com [40.107.20.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 51D13D4A; Thu, 7 Dec 2023 03:51:20 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=EVAk7iX9ZdNiXQtln7jLgNqgGtb34MAO7F6OWMqOeG9EffhnmUORAQU073SKcRgCDEjGEuRSxFKnIz0qEc68tpo62XS/RpmfI+R90y/PlJOEjG4RM3sSw92zZuFB0/4N94SWFfLPiQJ1yLJ7P0eMK00CwntxNo86GVeF3b5Uy1Aqs7EWSPstiqRXWuM1c9M7Uupk46JQEnV8fymoHjWlNs9Y7Tul4H2jTJGDbITldIh5ykMKbRxWWzxgCHSOkuOSEHBynI4LDQMfVyogByQBIPF1g4uQDbsaiJ1IYH/tFMzvXGbqiATeCykykY27/YX0FxCeRPXCNRoMAGxss6hbeQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=XOVUwsCdesOAxIwKuBpk7IheoIyYIMKYs3+/KWhnNw8=; b=J0FXJkGwiiq9hsNhDrxkipH/JVLJJDHAANLJAIiJv532WiYbsxFDt7JkyAhOLo6eOtf91Cg5fmHgcHOCNX8t74QnzdgvooHgoC5Q6+/CWzZrNaDdMFqCYv2GyxHLnCSXTg55bUCyEZjWA0s5oD00IBi0G7ztfhj018EJN6yfTb+dRAIKaehF483K2ojBEoyje6ARjYHYsrvtk8HFrc+FriL/LSy7e/L++2ohge1P+IRtefAuyZ6ghSAMFTSbeCXCxLgRIkTTIs3ugrpgIaBziv20ihE+zqNXrl37qISPC9ZE7NvygGqm0mC5atutFyOUrHeSE5gUDXhNi+z7B987Sw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=wolfvision.net; dmarc=pass action=none header.from=wolfvision.net; dkim=pass header.d=wolfvision.net; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wolfvision.net; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=XOVUwsCdesOAxIwKuBpk7IheoIyYIMKYs3+/KWhnNw8=; b=yT73oPMJmwnzrlNd9y97Wo05beUTMeBsbGc9Gh9mgCS4I1vtgrko2uKoLr+skYQy/S0cXJhXA3eYjB4x5D4rsuAQKra0ljqjga2kNKOIBGYxug2y3zyqPTdO2WEZEo/IAHBTK/sZaSO/DN1iyzlZzyPRfh5I4YlwIX7Xpnp0WxI= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=wolfvision.net; Received: from VE1PR08MB4974.eurprd08.prod.outlook.com (2603:10a6:803:111::15) by DB4PR08MB8054.eurprd08.prod.outlook.com (2603:10a6:10:389::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7046.34; Thu, 7 Dec 2023 11:51:16 +0000 Received: from VE1PR08MB4974.eurprd08.prod.outlook.com ([fe80::6b40:1e6f:7c94:71dc]) by VE1PR08MB4974.eurprd08.prod.outlook.com ([fe80::6b40:1e6f:7c94:71dc%4]) with mapi id 15.20.7068.027; Thu, 7 Dec 2023 11:51:16 +0000 From: Javier Carrasco <javier.carrasco@wolfvision.net> Date: Thu, 07 Dec 2023 12:51:07 +0100 Subject: [PATCH 2/4] usb: typec: tipd: add function to request firmware Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20231207-tps6598x_update-v1-2-dc21b5301d91@wolfvision.net> References: <20231207-tps6598x_update-v1-0-dc21b5301d91@wolfvision.net> In-Reply-To: <20231207-tps6598x_update-v1-0-dc21b5301d91@wolfvision.net> To: Heikki Krogerus <heikki.krogerus@linux.intel.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Javier Carrasco <javier.carrasco@wolfvision.net> X-Mailer: b4 0.12.0 X-Developer-Signature: v=1; a=ed25519-sha256; t=1701949872; l=2038; i=javier.carrasco@wolfvision.net; s=20230509; h=from:subject:message-id; bh=IiE6db3v4YrX0nZYmhvzOKALUG0QQqEAuGZEbigpqME=; b=TrqZJpl0BlUS3GCONO2c/i7kZj3SUO5wEvD22r/kf6tYVFUoSEiJPOPzy+1Exxc2c/ga9xIVx PP2PCmGyGO9ClSBkFbO9YcDs4YxKGTSzxGDpIFKz8PXTDmpzCpcUX/5 X-Developer-Key: i=javier.carrasco@wolfvision.net; a=ed25519; pk=tIGJV7M+tCizagNijF0eGMBGcOsPD+0cWGfKjl4h6K8= X-ClientProxiedBy: VI1PR08CA0248.eurprd08.prod.outlook.com (2603:10a6:803:dc::21) To VE1PR08MB4974.eurprd08.prod.outlook.com (2603:10a6:803:111::15) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: VE1PR08MB4974:EE_|DB4PR08MB8054:EE_ X-MS-Office365-Filtering-Correlation-Id: b890cb81-ad2a-4f87-8d18-08dbf71ad02b X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Fw9m00jCwL73MpgX/9RSdUSaTWjpW4xNNUntMDtGmqvL5Hh6mcTRfN08FurIIOOZEUctP5mGApQvClHNVvrHsNAbZCXv57r0eQLgj531jnqqWmLMYRzXcQia8XZqWtlwBRTC2DRSm1jsY2EcgRfoVS+HGcMS89K64s1H+XVMYnW0aCQ7XN+Uz8NzFmIsJYS/jpEGHUVbjoskz1kWGrntg9YKCI4xiW8jfOUO4DBZc0c0o7u4OKrxW23PewrH9AE37z12Pew0xbEK2GIZGyG7peM0rp5WjF0lLjj6sZkqFGd32eaH9AKsIXfT7hhNb7kkKuv1lGasRBKFmEkFEWo/+BNfsh9e4CIAT9ZtJX51h8Vdx2n5ogotlRqe6O4RA6DxeC/mqQzlqESz+KYhM7/F2W5OosdevGIqC+u8SWTtDvYMUQJnW/pAfaOSpVu0D19UT35jk7p0mFT1Ta09r+JF4LY4pB1ciVoKDGhqW4oJ33u7l+7VjI7QYvnFOwxDfHfVm2UmxOXFHF7Ke4RDvjhLYX7j4VkCII7RLkTE3ASL7Ha+mzsM2pemVz96MzAWsth9ck2aEQbSz8dkjz+8NPVAdb8rpp7FQqPgZR/H90YhihIm2z1xjWF/wkqnGtHDfbeo X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VE1PR08MB4974.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230031)(39840400004)(136003)(396003)(376002)(366004)(346002)(230922051799003)(1800799012)(186009)(64100799003)(451199024)(52116002)(38100700002)(5660300002)(38350700005)(2906002)(2616005)(6512007)(83380400001)(6666004)(6506007)(107886003)(26005)(110136005)(36756003)(6486002)(478600001)(41300700001)(44832011)(66946007)(66556008)(66476007)(86362001)(8936002)(4326008)(316002)(8676002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?q?nUYz1UL4jzqfd5+EPfIzKUrb2c50?= =?utf-8?q?Q6j6mYiGMJRpev+EgTUsYJiprF2ogB+PI9dkNEjG6qO9rzrNzKDSbHouHUOWbHlYF?= =?utf-8?q?kXSxyrsmrPPdR5ee4mjyz9S2F9LlZghc3jQSOwON0Ih7Lknc36QPLlm0BBF+zEQ92?= =?utf-8?q?IaXo6s763Ot3rDZjT4OszgZ2bhxP2DXP8l2Blbzlg/KCHIm5zl5v1tHbLReulEL2D?= =?utf-8?q?zI33Q8IyVwACgcAdfNM3TmynlmkHu7T73gYlXETVv+p5SpzR47ZS65J+dQo2VF2vd?= =?utf-8?q?8sXRM44aowaomYGfZ8Vjh3JYsaOFe8Of6BL0ZXnotgmAGdXUQzRPb1duaPHsh7ORW?= =?utf-8?q?QZTAfFLvRrtL/0I964v+42obx97MonZtZ+mhxDJX3J7D4ZRAgYbZsC34AQDNCmoqb?= =?utf-8?q?kawMUZe9+tXkdPGEwNMp+bdmGXJRUxM4ur3rV3vCOYIaO2O5exn9ZHrvqmPKvgadU?= =?utf-8?q?YOAp/eZZjePB4xoc/sMcP4TDBvIyPkK8xCeSNa4twMcJMAGRi+8hO8gsqnpq7XcDC?= =?utf-8?q?cdTzpbwKKBG6XTwM5clh42k0a95FXfJltk68ZCYHweCQE23Ieip8ouJwywgUoe5bF?= =?utf-8?q?FaGHXP7xoOKnlMLXPKLmCfhvM/iJF3aNcLv0zOzg0OjlLMaJMnIflKj1djaFbT0dI?= =?utf-8?q?AcnZL8/mUvUg65Y8oD0AUCeTr8DwYyTsG5YrhvVtOHDUumgIVY9CVOVfuQfM076r+?= =?utf-8?q?SRFJsOG7Ap6gHBdw78yuVLhgizXkk6KdnIjkMn6gqhOAi/DHChuCGdpSgVvbIqiCX?= =?utf-8?q?5hEyeKMkgTLH2e5rQCma2BpoPgVLCGXqvuXE6/fYy+wydOdpanehfIX8Ixlm6+F8n?= =?utf-8?q?GtobjV4jtCij9aJyVbOY8cRrvIcPMm+Rtk6X0qcTSwYftcjF0GIPo41RqlUWFXux/?= =?utf-8?q?4TMGV/PBNfYJ3pGdymJHSlVSi3OOwC/cWEcgmU+m/LkRIF1d9XF5WifYIsNJDYHad?= =?utf-8?q?QjwM95FHFfaR/NVXFl1mvnO7/n1eEPWUNW/I3cYyc14p5XThDDbRqTnvUCwKLJgQx?= =?utf-8?q?UdOeYkVoNlnf5yz19yVXDZqS8Zbf4DAdUed5tlyAO3KKf1KRax2Y5DDTSnUy4xasm?= =?utf-8?q?kQJxd6OwWfoQoBpXIYo5wgwGOSqelXEGvw7+pkW3+wUPwR4y0/mbDWPjdFgAT0d44?= =?utf-8?q?kOEUAhexjsR9iJeDhkyZogrTzcp3ZuVIABA0bPt9ac+bEwUt7bUpZpZGEgecZQse/?= =?utf-8?q?cmQN4V1tn4RTMx4HBkWv30aSKkmdhb/Eo9z9vJ1kOvCer5IMgxy6toHAkCnJtav3S?= =?utf-8?q?nfax10lXgUKrOBKJ9yPTSjdUx4icwpWIPYO9JAfZRgHNY7egSwzUaTpDX5WX5V7xG?= =?utf-8?q?c1Ol2vJjvc1Z5ykvchXK7hyetr4l26aBRz3z9PfAPKcljRebnfyeWbKIIynFjMw3c?= =?utf-8?q?TnhhaZZdjdBq8O5JgZ/Av5F6Fd/KeuH/ko2CJR7fXMJH4usHZZ2xN4xm+HePjx5+R?= =?utf-8?q?LO8nb8jlosUQcmw/2TaxgTar2zdEI19zYWfXFAGM2ux9P+Wrpae62FvMSCE0+tOWB?= =?utf-8?q?XLQUzet9VNhmuAaD4BEQ4Z+phb5O1omeaptzM4pqBtRTEiiWyJX4KQM=3D?= X-OriginatorOrg: wolfvision.net X-MS-Exchange-CrossTenant-Network-Message-Id: b890cb81-ad2a-4f87-8d18-08dbf71ad02b X-MS-Exchange-CrossTenant-AuthSource: VE1PR08MB4974.eurprd08.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Dec 2023 11:51:13.7891 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: e94ec9da-9183-471e-83b3-51baa8eb804f X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: HTCWi00nD4a625C3drVdZs+QXipLfrXZG8Jqk0hO6sQMiHj2PtMK2eekLddUGwWHSwW3iolWzy1jNv+xPxb0ZtowvgucvxWFrDx6rfJiUmI= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB4PR08MB8054 X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Thu, 07 Dec 2023 03:51:36 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784623816639260169 X-GMAIL-MSGID: 1784623816639260169 |
Series |
usb: typec: tipd: add patch update support for tps6598x
|
|
Commit Message
Javier Carrasco
Dec. 7, 2023, 11:51 a.m. UTC
The firmware request process is device agnostic and can be used for
other parts.
A probe deferring mechanism has been added in order to account for cases
where the file system where the firmware resides is still not available
when the probe function is triggered and no firmware is found.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
drivers/usb/typec/tipd/core.c | 36 +++++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
Comments
Hi Javier, On Thu, Dec 07, 2023 at 12:51:07PM +0100, Javier Carrasco wrote: > The firmware request process is device agnostic and can be used for > other parts. > > A probe deferring mechanism has been added in order to account for cases > where the file system where the firmware resides is still not available > when the probe function is triggered and no firmware is found. > > Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> > --- > drivers/usb/typec/tipd/core.c | 36 +++++++++++++++++++++++++++--------- > 1 file changed, 27 insertions(+), 9 deletions(-) > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c > index f0c4cd571a37..165a1391dc72 100644 > --- a/drivers/usb/typec/tipd/core.c > +++ b/drivers/usb/typec/tipd/core.c > @@ -873,6 +873,31 @@ tps6598x_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode) > return 0; > } > > +static int tps_request_firmware(struct tps6598x *tps, const struct firmware **fw) > +{ > + const char *firmware_name; > + int ret; > + > + ret = device_property_read_string(tps->dev, "firmware-name", > + &firmware_name); > + if (ret) > + return ret; > + > + ret = request_firmware(fw, firmware_name, tps->dev); > + if (ret) { > + dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name); > + /* probe deferring in case the file system is not ready */ > + return (ret == -ENOENT) ? -EPROBE_DEFER : ret; It's more likely that the firmware really isn't available, and it will never be available in this case. I think there is only one place in kernel where failing request_firmware() can lead to deferred probe (drivers/tee/optee/smc_abi.c) and there the code can actually see the system state - that's actually the condition. So just return dev_err_probe() here: ret = request_firmware(fw, firmware_name, tps->dev); if (ret) return dev_err_probe(tps->dev, ret, "failed to retrieve \"%s\"", firmware_name); > + } > + > + if ((*fw)->size == 0) { > + release_firmware(*fw); > + ret = -EINVAL; > + } > + > + return ret; > +} > + > static int > tps25750_write_firmware(struct tps6598x *tps, > u8 bpms_addr, const u8 *data, size_t len) > @@ -961,16 +986,9 @@ static int tps25750_start_patch_burst_mode(struct tps6598x *tps) > if (ret) > return ret; > > - ret = request_firmware(&fw, firmware_name, tps->dev); > - if (ret) { > - dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name); > + ret = tps_request_firmware(tps, &fw); > + if (ret) > return ret; > - } > - > - if (fw->size == 0) { > - ret = -EINVAL; > - goto release_fw; > - } > > ret = of_property_match_string(np, "reg-names", "patch-address"); > if (ret < 0) { > > -- > 2.39.2 thanks,
Hi Heikki, On 08.12.23 15:55, Heikki Krogerus wrote: >> + ret = request_firmware(fw, firmware_name, tps->dev); >> + if (ret) { >> + dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name); >> + /* probe deferring in case the file system is not ready */ >> + return (ret == -ENOENT) ? -EPROBE_DEFER : ret; > > It's more likely that the firmware really isn't available, and it will > never be available in this case. I think there is only one place in > kernel where failing request_firmware() can lead to deferred probe > (drivers/tee/optee/smc_abi.c) and there the code can actually see the > system state - that's actually the condition. > > So just return dev_err_probe() here: > > ret = request_firmware(fw, firmware_name, tps->dev); > if (ret) > return dev_err_probe(tps->dev, ret, "failed to retrieve \"%s\"", firmware_name); > Thank you for your feedback. This solution arose from a real use case: in the system I am using to test the tps65987d, the filesystem is not ready when the probe function is called. If I just return on -ENOENT, the device will never get the update. Note that we are only triggering the update if the device is in patch mode, so a firmware will be expected for the device to run and reach the app mode. In that case deferring the probe and keeping on trying to make the update makes sense because otherwise the device will not be able to offer its functionality. If the device is not in patch mode, no update will be triggered and the firmware will not be requested, so there will not be any unnecessary probe deferring. I see that the driver you mentioned checks if the system_state is still not SYSTEM_RUNNING to defer the probe. I have not tested if something like that would be possible in this case, but giving up on the first attempt if the firmware is not found makes the assumption that the filesystem where the fw resides will always be ready when the probe function is called, which in my particular case is a wrong assumption. If the firmware was updated at any point during normal operation, the assumption would be definitely right, but maybe not while booting. Thank you and best regards, Javier Carrasco
Hi, On Fri, Dec 08, 2023 at 07:58:52PM +0100, Javier Carrasco wrote: > Hi Heikki, > > On 08.12.23 15:55, Heikki Krogerus wrote: > > >> + ret = request_firmware(fw, firmware_name, tps->dev); > >> + if (ret) { > >> + dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name); > >> + /* probe deferring in case the file system is not ready */ > >> + return (ret == -ENOENT) ? -EPROBE_DEFER : ret; > > > > It's more likely that the firmware really isn't available, and it will > > never be available in this case. I think there is only one place in > > kernel where failing request_firmware() can lead to deferred probe > > (drivers/tee/optee/smc_abi.c) and there the code can actually see the > > system state - that's actually the condition. > > > > So just return dev_err_probe() here: > > > > ret = request_firmware(fw, firmware_name, tps->dev); > > if (ret) > > return dev_err_probe(tps->dev, ret, "failed to retrieve \"%s\"", firmware_name); > > > Thank you for your feedback. > > This solution arose from a real use case: in the system I am using to > test the tps65987d, the filesystem is not ready when the probe function > is called. If I just return on -ENOENT, the device will never get the > update. Just like all the other devices that require firmware. This driver is no different from the others, and it is also not the only one that needs the firmware only in special cases. Just make the firmware part of your ramdisk, or build the driver as a module. Are these firmwares available linux-firmware (or are the going to be)? https://git.kernel.org/?p=linux/kernel/git/firmware/linux-firmware.git thanks,
On 12.12.23 15:15, Heikki Krogerus wrote: > Hi, > > On Fri, Dec 08, 2023 at 07:58:52PM +0100, Javier Carrasco wrote: >> Hi Heikki, >> >> On 08.12.23 15:55, Heikki Krogerus wrote: >> >>>> + ret = request_firmware(fw, firmware_name, tps->dev); >>>> + if (ret) { >>>> + dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name); >>>> + /* probe deferring in case the file system is not ready */ >>>> + return (ret == -ENOENT) ? -EPROBE_DEFER : ret; >>> >>> It's more likely that the firmware really isn't available, and it will >>> never be available in this case. I think there is only one place in >>> kernel where failing request_firmware() can lead to deferred probe >>> (drivers/tee/optee/smc_abi.c) and there the code can actually see the >>> system state - that's actually the condition. >>> >>> So just return dev_err_probe() here: >>> >>> ret = request_firmware(fw, firmware_name, tps->dev); >>> if (ret) >>> return dev_err_probe(tps->dev, ret, "failed to retrieve \"%s\"", firmware_name); >>> >> Thank you for your feedback. >> >> This solution arose from a real use case: in the system I am using to >> test the tps65987d, the filesystem is not ready when the probe function >> is called. If I just return on -ENOENT, the device will never get the >> update. > > Just like all the other devices that require firmware. This driver is > no different from the others, and it is also not the only one that > needs the firmware only in special cases. Just make the firmware part > of your ramdisk, or build the driver as a module. I wonder why then there is no general solution that does not force the driver to be built as a module. If there is none, the documentation should mention that somehow (sorry if it does, I missed it). Actually a solution like the one implemented in the driver you mentioned could be used by any driver that can wait to be updated when the system is running. > Are these firmwares available linux-firmware (or are the going to be)? > https://git.kernel.org/?p=linux/kernel/git/firmware/linux-firmware.git > > thanks, > The firmware (at least for the tps6598x) can be tailored with a TI specific tool and it depends on the use case, so I suppose making it public does not make much sense. Best regards, Javier Carrasco
On Tue, Dec 12, 2023 at 03:41:35PM +0100, Javier Carrasco wrote: > > > On 12.12.23 15:15, Heikki Krogerus wrote: > > Hi, > > > > On Fri, Dec 08, 2023 at 07:58:52PM +0100, Javier Carrasco wrote: > >> Hi Heikki, > >> > >> On 08.12.23 15:55, Heikki Krogerus wrote: > >> > >>>> + ret = request_firmware(fw, firmware_name, tps->dev); > >>>> + if (ret) { > >>>> + dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name); > >>>> + /* probe deferring in case the file system is not ready */ > >>>> + return (ret == -ENOENT) ? -EPROBE_DEFER : ret; > >>> > >>> It's more likely that the firmware really isn't available, and it will > >>> never be available in this case. I think there is only one place in > >>> kernel where failing request_firmware() can lead to deferred probe > >>> (drivers/tee/optee/smc_abi.c) and there the code can actually see the > >>> system state - that's actually the condition. > >>> > >>> So just return dev_err_probe() here: > >>> > >>> ret = request_firmware(fw, firmware_name, tps->dev); > >>> if (ret) > >>> return dev_err_probe(tps->dev, ret, "failed to retrieve \"%s\"", firmware_name); > >>> > >> Thank you for your feedback. > >> > >> This solution arose from a real use case: in the system I am using to > >> test the tps65987d, the filesystem is not ready when the probe function > >> is called. If I just return on -ENOENT, the device will never get the > >> update. > > > > Just like all the other devices that require firmware. This driver is > > no different from the others, and it is also not the only one that > > needs the firmware only in special cases. Just make the firmware part > > of your ramdisk, or build the driver as a module. > > I wonder why then there is no general solution that does not force the > driver to be built as a module. Why would you need anything like that? Are you saying that even if you put the firmware into your ramdisk, the driver still fails to find the firmware if it's statically build? If so, then there is something else wrong. > If there is none, the documentation > should mention that somehow (sorry if it does, I missed it). Actually a > solution like the one implemented in the driver you mentioned could be > used by any driver that can wait to be updated when the system is > running. > > > Are these firmwares available linux-firmware (or are the going to be)? > > https://git.kernel.org/?p=linux/kernel/git/firmware/linux-firmware.git > > > > thanks, > > > The firmware (at least for the tps6598x) can be tailored with a TI > specific tool and it depends on the use case, so I suppose making it > public does not make much sense. Okay. thanks,
On 13.12.23 16:15, Heikki Krogerus wrote: > On Tue, Dec 12, 2023 at 03:41:35PM +0100, Javier Carrasco wrote: >> I wonder why then there is no general solution that does not force the >> driver to be built as a module. > > Why would you need anything like that? Are you saying that even if you > put the firmware into your ramdisk, the driver still fails to find the > firmware if it's statically build? If so, then there is something else > wrong. > The firmware is always found unless the file system is still not ready, which is the case on the system I am working on. If the driver is built as a module, the issue is gone as expected. My point was that there is no limitation to have the driver built-in and no documentation to reference, so anyone could stumble on the same issue. And as you said, this driver is not special in that sense, so other drivers might be facing the same eventuality. Am I missing any existing documentation for the fact that the firmware must be put into the ramdisk or the driver must be built as a module? Or is it only based on common sense? Anyway the next version will not have any probe deferring and only return an error if the firmware is not available. Thanks and best regards, Javier Carrasco
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c index f0c4cd571a37..165a1391dc72 100644 --- a/drivers/usb/typec/tipd/core.c +++ b/drivers/usb/typec/tipd/core.c @@ -873,6 +873,31 @@ tps6598x_register_port(struct tps6598x *tps, struct fwnode_handle *fwnode) return 0; } +static int tps_request_firmware(struct tps6598x *tps, const struct firmware **fw) +{ + const char *firmware_name; + int ret; + + ret = device_property_read_string(tps->dev, "firmware-name", + &firmware_name); + if (ret) + return ret; + + ret = request_firmware(fw, firmware_name, tps->dev); + if (ret) { + dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name); + /* probe deferring in case the file system is not ready */ + return (ret == -ENOENT) ? -EPROBE_DEFER : ret; + } + + if ((*fw)->size == 0) { + release_firmware(*fw); + ret = -EINVAL; + } + + return ret; +} + static int tps25750_write_firmware(struct tps6598x *tps, u8 bpms_addr, const u8 *data, size_t len) @@ -961,16 +986,9 @@ static int tps25750_start_patch_burst_mode(struct tps6598x *tps) if (ret) return ret; - ret = request_firmware(&fw, firmware_name, tps->dev); - if (ret) { - dev_err(tps->dev, "failed to retrieve \"%s\"\n", firmware_name); + ret = tps_request_firmware(tps, &fw); + if (ret) return ret; - } - - if (fw->size == 0) { - ret = -EINVAL; - goto release_fw; - } ret = of_property_match_string(np, "reg-names", "patch-address"); if (ret < 0) {