From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9C1DC54EBE for ; Sat, 7 Jan 2023 09:16:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230512AbjAGJQt (ORCPT ); Sat, 7 Jan 2023 04:16:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60612 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229815AbjAGJQs (ORCPT ); Sat, 7 Jan 2023 04:16:48 -0500 Received: from mail-il1-x136.google.com (mail-il1-x136.google.com [IPv6:2607:f8b0:4864:20::136]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6D86584098 for ; Sat, 7 Jan 2023 01:16:47 -0800 (PST) Received: by mail-il1-x136.google.com with SMTP id u8so2226700ilg.0 for ; Sat, 07 Jan 2023 01:16:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=RW3tgbxJ4a2R6Lr3hH31QiBztWxbB+zFWSFDhcB5PzA=; b=KKfAvH6c6JEgsJOT6JY3oShOZFTY6igTfAUjUAfOvO3ZTBJILFDvW/q3p4JG0BSbDr K+YbSwq/zOnsJrBIbou5SGfbimt6dioyluqbq/pPcOoA+YAVcadHQdaZXpG9VwkfjrXq p0GnhAoCy+/LFON2ea16s1HP+P1sjTyceRnXM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RW3tgbxJ4a2R6Lr3hH31QiBztWxbB+zFWSFDhcB5PzA=; b=cHZzlTELe0Jkj4yePQSbJAWBY+iGNifV06j9hfgtB9lMfVezLspwEUW/TRGmWghmYb lJM3HcfhHrrWIX4OR+lu1Vovp4sh7QHajTX3bGHDPryvPXlOZ2rtAo+t3SmGo0PWhbvW AWkLATMn0F1J7i+01ziJaH+QVFE6HoWQWXdLedAiFK0Nh2YLPxUm73tH7OKBys9sOsps DM9FzrVbCWj2R/HMRUjAepYpJ7MOu5wXHuPqTKVmt+9iPkOUU9n9Jqi/tX/iVJ5b6RxW 5TZCdKx8UJr8u1n1FzXpDQ5nWw7sqfp1hsIi9+SdzOel/tr7DRIS9w5BCdNHGHT6RD9m 5rrw== X-Gm-Message-State: AFqh2kqqr+Ut07yD2WIDJ9paAwV+HgSxtKnUBRlvtDU6Lco0v6pabEE5 JGgpHsSVzDXFaNqNcwRApEi20WLcChIRTE4BaUJXoA== X-Google-Smtp-Source: AMrXdXtBMzB4lO1Yn6qSZ1CSDd89k7nZTSy0I48BAfgZ30l48dPrz+xuh/08ZScwMOp5MGwUZ1rNJl0wH1TYIiXt25E= X-Received: by 2002:a05:6e02:2141:b0:30d:8aeb:9b11 with SMTP id d1-20020a056e02214100b0030d8aeb9b11mr588524ilv.293.1673083006788; Sat, 07 Jan 2023 01:16:46 -0800 (PST) MIME-Version: 1.0 References: <20230105132457.4125372-1-treapking@chromium.org> <20230105132457.4125372-4-treapking@chromium.org> In-Reply-To: From: Pin-yen Lin Date: Sat, 7 Jan 2023 17:16:35 +0800 Message-ID: Subject: Re: [PATCH v7 3/9] drm/display: Add Type-C switch helpers To: Andy Shevchenko Cc: Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , David Airlie , Daniel Vetter , Rob Herring , Krzysztof Kozlowski , Daniel Scally , Heikki Krogerus , Sakari Ailus , Greg Kroah-Hartman , "Rafael J . Wysocki" , Prashant Malani , Benson Leung , Guenter Roeck , =?UTF-8?B?TsOtY29sYXMgRiAuIFIgLiBBIC4gUHJhZG8=?= , Xin Ji , AngeloGioacchino Del Regno , Thomas Zimmermann , Hsin-Yi Wang , linux-kernel@vger.kernel.org, Allen Chen , linux-acpi@vger.kernel.org, Lyude Paul , dri-devel@lists.freedesktop.org, chrome-platform@lists.linux.dev, Javier Martinez Canillas , Marek Vasut , devicetree@vger.kernel.org, Stephen Boyd , Douglas Anderson , Imre Deak , Jani Nikula , Kees Cook , Khaled Almahallawy , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , shaomin Deng Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Hi Andy, Thanks for the review. On Thu, Jan 5, 2023 at 11:41 PM Andy Shevchenko wrote: > > On Thu, Jan 05, 2023 at 09:24:51PM +0800, Pin-yen Lin wrote: > > Add helpers to register and unregister Type-C "switches" for bridges > > capable of switching their output between two downstream devices. > > > > The helper registers USB Type-C mode switches when the "mode-switch" > > and the "data-lanes" properties are available in Device Tree. > > ... > > > + port_data->typec_mux = typec_mux_register(dev, &mux_desc); > > + if (IS_ERR(port_data->typec_mux)) { > > + ret = PTR_ERR(port_data->typec_mux); > > + dev_err(dev, "Mode switch register for port %d failed: %d\n", > > + port_num, ret); > > + } > > + > > + return ret; > > ... > > > + struct device_node *sw; > > > + int ret = 0; > > It's easy to break things if you squeeze more code in the future in this > function, so I recommend to split assignment to be closer to its first user > (see below). > > > + for_each_child_of_node(port, sw) { > > + if (of_property_read_bool(sw, "mode-switch")) > > + switch_desc->num_typec_switches++; > > + } > > + > > + if (!switch_desc->num_typec_switches) { > > + dev_warn(dev, "No Type-C switches node found\n"); > > > + return ret; > > return 0; Thanks for the suggestion. I'll update this in v8. > > > + } > > + > > + switch_desc->typec_ports = devm_kcalloc( > > + dev, switch_desc->num_typec_switches, > > + sizeof(struct drm_dp_typec_port_data), GFP_KERNEL); > > + > > + if (!switch_desc->typec_ports) > > + return -ENOMEM; > > > + /* Register switches for each connector. */ > > + for_each_child_of_node(port, sw) { > > + if (!of_property_read_bool(sw, "mode-switch")) > > + continue; > > + ret = drm_dp_register_mode_switch(dev, sw, switch_desc, data, mux_set); > > + if (ret) { > > + dev_err(dev, "Failed to register mode switch: %d\n", ret); > > + of_node_put(sw); > > + break; > > + } > > + } > > > + if (ret) > > + drm_dp_unregister_typec_switches(switch_desc); > > + > > + return ret; > > Why not adding a goto label? I didn't know that goto label is preferred even when there are no duplicated code blocks in the function. I'll update this accordingly in v8. > > ret = drm_dp_register_mode_switch(dev, sw, switch_desc, data, mux_set); > if (ret) > goto err_unregister_typec_switches; > > return 0; > > err_unregister_typec_switches: > of_node_put(sw); > drm_dp_unregister_typec_switches(switch_desc); > dev_err(dev, "Failed to register mode switch: %d\n", ret); > return ret; > > -- > With Best Regards, > Andy Shevchenko > > Best regards, Pin-yen