From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Pin-yen Lin <treapking@chromium.org> Cc: "Andrzej Hajda" <andrzej.hajda@intel.com>, "Neil Armstrong" <neil.armstrong@linaro.org>, "Robert Foss" <robert.foss@linaro.org>, "Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>, "Jonas Karlman" <jonas@kwiboo.se>, "Jernej Skrabec" <jernej.skrabec@gmail.com>, "David Airlie" <airlied@gmail.com>, "Daniel Vetter" <daniel@ffwll.ch>, "Rob Herring" <robh+dt@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Daniel Scally" <djrscally@gmail.com>, "Heikki Krogerus" <heikki.krogerus@linux.intel.com>, "Sakari Ailus" <sakari.ailus@linux.intel.com>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, "Rafael J . Wysocki" <rafael@kernel.org>, "Prashant Malani" <pmalani@chromium.org>, "Benson Leung" <bleung@chromium.org>, "Guenter Roeck" <groeck@chromium.org>, "Javier Martinez Canillas" <javierm@redhat.com>, "Stephen Boyd" <swboyd@chromium.org>, dri-devel@lists.freedesktop.org, "Hsin-Yi Wang" <hsinyi@chromium.org>, "Thomas Zimmermann" <tzimmermann@suse.de>, devicetree@vger.kernel.org, chrome-platform@lists.linux.dev, linux-acpi@vger.kernel.org, "Marek Vasut" <marex@denx.de>, "Xin Ji" <xji@analogixsemi.com>, "Lyude Paul" <lyude@redhat.com>, "Nícolas F . R . A . Prado" <nfraprado@collabora.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>, linux-kernel@vger.kernel.org, "Allen Chen" <allen.chen@ite.com.tw> Subject: Re: [PATCH v6 5/7] drm/bridge: anx7625: Register Type C mode switches Date: Thu, 24 Nov 2022 14:18:04 +0200 [thread overview] Message-ID: <Y39g/OBLW3pt9qdy@smile.fi.intel.com> (raw) In-Reply-To: <20221124102056.393220-6-treapking@chromium.org> On Thu, Nov 24, 2022 at 06:20:54PM +0800, Pin-yen Lin wrote: > Register USB Type-C mode switches when the "mode-switch" property and > relevant port are available in Device Tree. Configure the crosspoint > switch based on the entered alternate mode for a specific Type-C > connector. ... > +static void anx7625_typec_two_ports_update(struct anx7625_data *ctx) > +{ > + if (ctx->typec_ports[0].dp_connected && ctx->typec_ports[1].dp_connected) > + /* Both ports available, do nothing to retain the current one. */ > + return; > + else if (ctx->typec_ports[0].dp_connected) This 'else' is redundant. I would rewrite above as /* Check if both ports available and do nothing to retain the current one */ if (ctx->typec_ports[0].dp_connected && ctx->typec_ports[1].dp_connected) return; if (ctx->typec_ports[0].dp_connected) > + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL); > + else if (ctx->typec_ports[1].dp_connected) > + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE); > +} ... > + data->dp_connected = (state->alt && state->alt->svid == USB_TYPEC_DP_SID && > + state->alt->mode == USB_TYPEC_DP_MODE); Parentheses are not needed. ... > + /* > + * <0 1> refers to SSRX1/SSTX1, and <2 3> refers to SSRX2/SSTX2. > + */ > + for (i = 0; i < num_lanes; i++) { > + if (port_num != -1 && port_num != dp_lanes[i] / 2) { > + dev_err(dev, "Invalid data lane numbers\n"); > + return -EINVAL; > + } According to Rob Linux must not validate device tree. If you need it, use proper YAML schema. > + port_num = dp_lanes[i] / 2; > + } ... > + if (!ctx->num_typec_switches) { > + dev_warn(dev, "No Type-C switches node found\n"); > + return ret; Why not to return 0 explicitly? > + } ... > + ctx->typec_ports = devm_kcalloc( Broken indentation. > + dev, ctx->num_typec_switches, sizeof(struct anx7625_port_data), > + GFP_KERNEL); > + if (!ctx->typec_ports) > + return -ENOMEM; ... > +struct anx7625_port_data { > + bool dp_connected; You can save some bytes on some architectures if move this to be last field. > + struct typec_mux_dev *typec_mux; > + struct anx7625_data *ctx; > +}; -- With Best Regards, Andy Shevchenko
WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Pin-yen Lin <treapking@chromium.org> Cc: "Heikki Krogerus" <heikki.krogerus@linux.intel.com>, "Andrzej Hajda" <andrzej.hajda@intel.com>, "Rafael J . Wysocki" <rafael@kernel.org>, dri-devel@lists.freedesktop.org, "Laurent Pinchart" <Laurent.pinchart@ideasonboard.com>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Guenter Roeck" <groeck@chromium.org>, "Marek Vasut" <marex@denx.de>, chrome-platform@lists.linux.dev, "Javier Martinez Canillas" <javierm@redhat.com>, "Jernej Skrabec" <jernej.skrabec@gmail.com>, linux-acpi@vger.kernel.org, devicetree@vger.kernel.org, "Sakari Ailus" <sakari.ailus@linux.intel.com>, "Nícolas F . R . A . Prado" <nfraprado@collabora.com>, "Jonas Karlman" <jonas@kwiboo.se>, "Allen Chen" <allen.chen@ite.com.tw>, "Stephen Boyd" <swboyd@chromium.org>, "Rob Herring" <robh+dt@kernel.org>, "Hsin-Yi Wang" <hsinyi@chromium.org>, "Xin Ji" <xji@analogixsemi.com>, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>, "Neil Armstrong" <neil.armstrong@linaro.org>, "Thomas Zimmermann" <tzimmermann@suse.de>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, linux-kernel@vger.kernel.org, "Robert Foss" <robert.foss@linaro.org>, "Daniel Scally" <djrscally@gmail.com>, "Prashant Malani" <pmalani@chromium.org> Subject: Re: [PATCH v6 5/7] drm/bridge: anx7625: Register Type C mode switches Date: Thu, 24 Nov 2022 14:18:04 +0200 [thread overview] Message-ID: <Y39g/OBLW3pt9qdy@smile.fi.intel.com> (raw) In-Reply-To: <20221124102056.393220-6-treapking@chromium.org> On Thu, Nov 24, 2022 at 06:20:54PM +0800, Pin-yen Lin wrote: > Register USB Type-C mode switches when the "mode-switch" property and > relevant port are available in Device Tree. Configure the crosspoint > switch based on the entered alternate mode for a specific Type-C > connector. ... > +static void anx7625_typec_two_ports_update(struct anx7625_data *ctx) > +{ > + if (ctx->typec_ports[0].dp_connected && ctx->typec_ports[1].dp_connected) > + /* Both ports available, do nothing to retain the current one. */ > + return; > + else if (ctx->typec_ports[0].dp_connected) This 'else' is redundant. I would rewrite above as /* Check if both ports available and do nothing to retain the current one */ if (ctx->typec_ports[0].dp_connected && ctx->typec_ports[1].dp_connected) return; if (ctx->typec_ports[0].dp_connected) > + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL); > + else if (ctx->typec_ports[1].dp_connected) > + anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE); > +} ... > + data->dp_connected = (state->alt && state->alt->svid == USB_TYPEC_DP_SID && > + state->alt->mode == USB_TYPEC_DP_MODE); Parentheses are not needed. ... > + /* > + * <0 1> refers to SSRX1/SSTX1, and <2 3> refers to SSRX2/SSTX2. > + */ > + for (i = 0; i < num_lanes; i++) { > + if (port_num != -1 && port_num != dp_lanes[i] / 2) { > + dev_err(dev, "Invalid data lane numbers\n"); > + return -EINVAL; > + } According to Rob Linux must not validate device tree. If you need it, use proper YAML schema. > + port_num = dp_lanes[i] / 2; > + } ... > + if (!ctx->num_typec_switches) { > + dev_warn(dev, "No Type-C switches node found\n"); > + return ret; Why not to return 0 explicitly? > + } ... > + ctx->typec_ports = devm_kcalloc( Broken indentation. > + dev, ctx->num_typec_switches, sizeof(struct anx7625_port_data), > + GFP_KERNEL); > + if (!ctx->typec_ports) > + return -ENOMEM; ... > +struct anx7625_port_data { > + bool dp_connected; You can save some bytes on some architectures if move this to be last field. > + struct typec_mux_dev *typec_mux; > + struct anx7625_data *ctx; > +}; -- With Best Regards, Andy Shevchenko
next prev parent reply other threads:[~2022-11-24 12:18 UTC|newest] Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-24 10:20 [PATCH v6 0/7] Register Type-C mode-switch in DP bridge endpoints Pin-yen Lin 2022-11-24 10:20 ` Pin-yen Lin 2022-11-24 10:20 ` [PATCH v6 1/7] device property: Add remote endpoint to devcon matcher Pin-yen Lin 2022-11-24 10:20 ` Pin-yen Lin 2022-11-24 10:20 ` [PATCH v6 2/7] platform/chrome: cros_ec_typec: Purge blocking switch devlinks Pin-yen Lin 2022-11-24 10:20 ` Pin-yen Lin 2022-11-24 12:24 ` Andy Shevchenko 2022-11-24 12:24 ` Andy Shevchenko 2022-11-25 5:52 ` Pin-yen Lin 2022-11-25 5:52 ` Pin-yen Lin 2022-11-25 5:53 ` Prashant Malani 2022-11-25 5:53 ` Prashant Malani 2022-11-24 10:20 ` [PATCH v6 3/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support Pin-yen Lin 2022-11-24 10:20 ` Pin-yen Lin 2022-11-24 17:39 ` Rob Herring 2022-11-24 17:39 ` Rob Herring 2022-11-25 3:58 ` Pin-yen Lin 2022-11-25 3:58 ` Pin-yen Lin 2022-11-27 20:58 ` Krzysztof Kozlowski 2022-11-27 20:58 ` Krzysztof Kozlowski 2022-12-26 8:49 ` Pin-yen Lin 2022-12-26 8:49 ` Pin-yen Lin 2022-11-24 10:20 ` [PATCH v6 4/7] drm/bridge: anx7625: Check for Type-C during panel registration Pin-yen Lin 2022-11-24 10:20 ` Pin-yen Lin 2022-11-24 10:20 ` [PATCH v6 5/7] drm/bridge: anx7625: Register Type C mode switches Pin-yen Lin 2022-11-24 10:20 ` Pin-yen Lin 2022-11-24 12:18 ` Andy Shevchenko [this message] 2022-11-24 12:18 ` Andy Shevchenko 2022-11-25 6:58 ` Pin-yen Lin 2022-11-25 6:58 ` Pin-yen Lin 2022-11-24 10:20 ` [PATCH v6 6/7] dt/bindings: drm/bridge: it6505: Add mode-switch support Pin-yen Lin 2022-11-24 10:20 ` Pin-yen Lin 2022-11-24 17:39 ` Rob Herring 2022-11-24 17:39 ` Rob Herring 2022-11-25 3:58 ` Pin-yen Lin 2022-11-25 3:58 ` Pin-yen Lin 2022-11-27 21:02 ` Krzysztof Kozlowski 2022-11-27 21:02 ` Krzysztof Kozlowski 2022-12-26 8:57 ` Pin-yen Lin 2022-12-26 8:57 ` Pin-yen Lin 2022-11-24 10:20 ` [PATCH v6 7/7] drm/bridge: it6505: Register Type C mode switches Pin-yen Lin 2022-11-24 10:20 ` Pin-yen Lin 2022-11-24 12:23 ` Andy Shevchenko 2022-11-24 12:23 ` Andy Shevchenko 2022-11-25 6:55 ` Pin-yen Lin 2022-11-25 6:55 ` Pin-yen Lin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Y39g/OBLW3pt9qdy@smile.fi.intel.com \ --to=andriy.shevchenko@linux.intel.com \ --cc=Laurent.pinchart@ideasonboard.com \ --cc=airlied@gmail.com \ --cc=allen.chen@ite.com.tw \ --cc=andrzej.hajda@intel.com \ --cc=angelogioacchino.delregno@collabora.com \ --cc=bleung@chromium.org \ --cc=chrome-platform@lists.linux.dev \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=djrscally@gmail.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=gregkh@linuxfoundation.org \ --cc=groeck@chromium.org \ --cc=heikki.krogerus@linux.intel.com \ --cc=hsinyi@chromium.org \ --cc=javierm@redhat.com \ --cc=jernej.skrabec@gmail.com \ --cc=jonas@kwiboo.se \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lyude@redhat.com \ --cc=marex@denx.de \ --cc=neil.armstrong@linaro.org \ --cc=nfraprado@collabora.com \ --cc=pmalani@chromium.org \ --cc=rafael@kernel.org \ --cc=robert.foss@linaro.org \ --cc=robh+dt@kernel.org \ --cc=sakari.ailus@linux.intel.com \ --cc=swboyd@chromium.org \ --cc=treapking@chromium.org \ --cc=tzimmermann@suse.de \ --cc=xji@analogixsemi.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.