| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z4j5V8ACfJLG-7Oy@kuha.fi.intel.com>
Date: Thu, 16 Jan 2025 14:19:35 +0200
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Mathias Nyman <mathias.nyman@...el.com>,
Thierry Reding <thierry.reding@...il.com>,
Jonathan Hunter <jonathanh@...dia.com>,
Andrew Jeffery <andrew@...econstruct.com.au>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Claudiu Beznea <claudiu.beznea@...on.dev>,
Daniel Mack <daniel@...que.org>,
Haojian Zhuang <haojian.zhuang@...il.com>,
Robert Jarzmik <robert.jarzmik@...e.fr>,
Peter Chen <peter.chen@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>,
Alan Stern <stern@...land.harvard.edu>, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, usb-storage@...ts.one-eyed-alien.net
Subject: Re: [PATCH 3/6] USB: typec: Use str_enable_disable-like helpers
On Tue, Jan 14, 2025 at 09:05:36PM +0100, Krzysztof Kozlowski wrote:
> Replace ternary (condition ? "enable" : "disable") syntax with helpers
> from string_choices.h because:
> 1. Simple function call with one argument is easier to read. Ternary
> operator has three arguments and with wrapping might lead to quite
> long code.
> 2. Is slightly shorter thus also easier to read.
> 3. It brings uniformity in the text - same string.
> 4. Allows deduping by the linker, which results in a smaller binary
> file.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> ---
> drivers/usb/typec/class.c | 7 ++++---
> drivers/usb/typec/tcpm/fusb302.c | 24 +++++++++++-----------
> .../usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c | 3 ++-
> .../typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c | 3 ++-
> drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.c | 4 +++-
> drivers/usb/typec/tcpm/tcpm.c | 7 ++++---
> 6 files changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index d9d019cff01908eaa8dcb484a87147f3d9992bf3..9c76c3d0c6cff9c9b94ef35fb0cb4be0e395aad6 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -10,6 +10,7 @@
> #include <linux/mutex.h>
> #include <linux/property.h>
> #include <linux/slab.h>
> +#include <linux/string_choices.h>
> #include <linux/usb/pd_vdo.h>
> #include <linux/usb/typec_mux.h>
> #include <linux/usb/typec_retimer.h>
> @@ -361,7 +362,7 @@ active_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct typec_altmode *alt = to_typec_altmode(dev);
>
> - return sprintf(buf, "%s\n", alt->active ? "yes" : "no");
> + return sprintf(buf, "%s\n", str_yes_no(alt->active));
> }
>
> static ssize_t active_store(struct device *dev, struct device_attribute *attr,
> @@ -707,7 +708,7 @@ static ssize_t supports_usb_power_delivery_show(struct device *dev,
> {
> struct typec_partner *p = to_typec_partner(dev);
>
> - return sprintf(buf, "%s\n", p->usb_pd ? "yes" : "no");
> + return sprintf(buf, "%s\n", str_yes_no(p->usb_pd));
> }
> static DEVICE_ATTR_RO(supports_usb_power_delivery);
>
> @@ -1855,7 +1856,7 @@ static ssize_t vconn_source_show(struct device *dev,
> struct typec_port *port = to_typec_port(dev);
>
> return sprintf(buf, "%s\n",
> - port->vconn_role == TYPEC_SOURCE ? "yes" : "no");
> + str_yes_no(port->vconn_role == TYPEC_SOURCE));
> }
> static DEVICE_ATTR_RW(vconn_source);
>
> diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> index e2fe479e16ada018930ea0dbbf58ee37ce9a1990..f15c63d3a8f441569ec98302f5b241430d8e4547 100644
> --- a/drivers/usb/typec/tcpm/fusb302.c
> +++ b/drivers/usb/typec/tcpm/fusb302.c
> @@ -24,6 +24,7 @@
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/string.h>
> +#include <linux/string_choices.h>
> #include <linux/types.h>
> #include <linux/usb.h>
> #include <linux/usb/typec.h>
> @@ -733,7 +734,7 @@ static int tcpm_set_vconn(struct tcpc_dev *dev, bool on)
>
> mutex_lock(&chip->lock);
> if (chip->vconn_on == on) {
> - fusb302_log(chip, "vconn is already %s", on ? "On" : "Off");
> + fusb302_log(chip, "vconn is already %s", str_on_off(on));
> goto done;
> }
> if (on) {
> @@ -746,7 +747,7 @@ static int tcpm_set_vconn(struct tcpc_dev *dev, bool on)
> if (ret < 0)
> goto done;
> chip->vconn_on = on;
> - fusb302_log(chip, "vconn := %s", on ? "On" : "Off");
> + fusb302_log(chip, "vconn := %s", str_on_off(on));
> done:
> mutex_unlock(&chip->lock);
>
> @@ -761,7 +762,7 @@ static int tcpm_set_vbus(struct tcpc_dev *dev, bool on, bool charge)
>
> mutex_lock(&chip->lock);
> if (chip->vbus_on == on) {
> - fusb302_log(chip, "vbus is already %s", on ? "On" : "Off");
> + fusb302_log(chip, "vbus is already %s", str_on_off(on));
> } else {
> if (on)
> ret = regulator_enable(chip->vbus);
> @@ -769,15 +770,14 @@ static int tcpm_set_vbus(struct tcpc_dev *dev, bool on, bool charge)
> ret = regulator_disable(chip->vbus);
> if (ret < 0) {
> fusb302_log(chip, "cannot %s vbus regulator, ret=%d",
> - on ? "enable" : "disable", ret);
> + str_enable_disable(on), ret);
> goto done;
> }
> chip->vbus_on = on;
> - fusb302_log(chip, "vbus := %s", on ? "On" : "Off");
> + fusb302_log(chip, "vbus := %s", str_on_off(on));
> }
> if (chip->charge_on == charge)
> - fusb302_log(chip, "charge is already %s",
> - charge ? "On" : "Off");
> + fusb302_log(chip, "charge is already %s", str_on_off(charge));
> else
> chip->charge_on = charge;
>
> @@ -854,16 +854,16 @@ static int tcpm_set_pd_rx(struct tcpc_dev *dev, bool on)
> ret = fusb302_pd_set_auto_goodcrc(chip, on);
> if (ret < 0) {
> fusb302_log(chip, "cannot turn %s auto GCRC, ret=%d",
> - on ? "on" : "off", ret);
> + str_on_off(on), ret);
> goto done;
> }
> ret = fusb302_pd_set_interrupts(chip, on);
> if (ret < 0) {
> fusb302_log(chip, "cannot turn %s pd interrupts, ret=%d",
> - on ? "on" : "off", ret);
> + str_on_off(on), ret);
> goto done;
> }
> - fusb302_log(chip, "pd := %s", on ? "on" : "off");
> + fusb302_log(chip, "pd := %s", str_on_off(on));
> done:
> mutex_unlock(&chip->lock);
>
> @@ -1531,7 +1531,7 @@ static void fusb302_irq_work(struct work_struct *work)
> if (interrupt & FUSB_REG_INTERRUPT_VBUSOK) {
> vbus_present = !!(status0 & FUSB_REG_STATUS0_VBUSOK);
> fusb302_log(chip, "IRQ: VBUS_OK, vbus=%s",
> - vbus_present ? "On" : "Off");
> + str_on_off(vbus_present));
> if (vbus_present != chip->vbus_present) {
> chip->vbus_present = vbus_present;
> tcpm_vbus_change(chip->tcpm_port);
> @@ -1562,7 +1562,7 @@ static void fusb302_irq_work(struct work_struct *work)
> if ((interrupt & FUSB_REG_INTERRUPT_COMP_CHNG) && intr_comp_chng) {
> comp_result = !!(status0 & FUSB_REG_STATUS0_COMP);
> fusb302_log(chip, "IRQ: COMP_CHNG, comp=%s",
> - comp_result ? "true" : "false");
> + str_true_false(comp_result));
> if (comp_result) {
> /* cc level > Rd_threshold, detach */
> chip->cc1 = TYPEC_CC_OPEN;
> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> index 726423684bae0a690bd20547313704b7d2f4cfdc..18303b34594bbf6f43d1138177c4ab58f0dec395 100644
> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy.c
> @@ -12,6 +12,7 @@
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> +#include <linux/string_choices.h>
> #include <linux/usb/pd.h>
> #include <linux/usb/tcpm.h>
> #include "qcom_pmic_typec.h"
> @@ -418,7 +419,7 @@ static int qcom_pmic_typec_pdphy_set_pd_rx(struct tcpc_dev *tcpc, bool on)
>
> spin_unlock_irqrestore(&pmic_typec_pdphy->lock, flags);
>
> - dev_dbg(pmic_typec_pdphy->dev, "set_pd_rx: %s\n", on ? "on" : "off");
> + dev_dbg(pmic_typec_pdphy->dev, "set_pd_rx: %s\n", str_on_off(on));
>
> return ret;
> }
> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c
> index df79059cda6755d4de35b7239fadc2dff2e699b1..8fac171778daf471fe4d03de8cc4f9c7ce1f323b 100644
> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c
> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_pdphy_stub.c
> @@ -12,6 +12,7 @@
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> +#include <linux/string_choices.h>
> #include <linux/usb/pd.h>
> #include <linux/usb/tcpm.h>
> #include "qcom_pmic_typec.h"
> @@ -38,7 +39,7 @@ static int qcom_pmic_typec_pdphy_stub_set_pd_rx(struct tcpc_dev *tcpc, bool on)
> struct pmic_typec *tcpm = tcpc_to_tcpm(tcpc);
> struct device *dev = tcpm->dev;
>
> - dev_dbg(dev, "set_pd_rx: %s\n", on ? "on" : "off");
> + dev_dbg(dev, "set_pd_rx: %s\n", str_on_off(on));
>
> return 0;
> }
> diff --git a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.c b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.c
> index c37dede62e12cd8a105da108838b5ca4f5e632d7..4fc83dcfae643e7a8b8e89ac6e6f5a9aaba3f65b 100644
> --- a/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.c
> +++ b/drivers/usb/typec/tcpm/qcom/qcom_pmic_typec_port.c
> @@ -13,6 +13,7 @@
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
> +#include <linux/string_choices.h>
> #include <linux/usb/tcpm.h>
> #include <linux/usb/typec_mux.h>
> #include <linux/workqueue.h>
> @@ -562,7 +563,8 @@ static int qcom_pmic_typec_port_set_vconn(struct tcpc_dev *tcpc, bool on)
> spin_unlock_irqrestore(&pmic_typec_port->lock, flags);
>
> dev_dbg(dev, "set_vconn: orientation %d control 0x%08x state %s cc %s vconn %s\n",
> - orientation, value, on ? "on" : "off", misc_to_vconn(misc), misc_to_cc(misc));
> + orientation, value, str_on_off(on), misc_to_vconn(misc),
> + misc_to_cc(misc));
>
> return ret;
> }
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index fcf2d7902352c447651f30610d59fef2471f3124..ef2cec386d965512c64e8b7e640199e10bb7bc94 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -21,6 +21,7 @@
> #include <linux/seq_file.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> +#include <linux/string_choices.h>
> #include <linux/usb.h>
> #include <linux/usb/pd.h>
> #include <linux/usb/pd_ado.h>
> @@ -892,8 +893,8 @@ static int tcpm_enable_auto_vbus_discharge(struct tcpm_port *port, bool enable)
>
> if (port->tcpc->enable_auto_vbus_discharge) {
> ret = port->tcpc->enable_auto_vbus_discharge(port->tcpc, enable);
> - tcpm_log_force(port, "%s vbus discharge ret:%d", enable ? "enable" : "disable",
> - ret);
> + tcpm_log_force(port, "%s vbus discharge ret:%d",
> + str_enable_disable(enable), ret);
> if (!ret)
> port->auto_vbus_discharge_enabled = enable;
> }
> @@ -4439,7 +4440,7 @@ static void tcpm_unregister_altmodes(struct tcpm_port *port)
>
> static void tcpm_set_partner_usb_comm_capable(struct tcpm_port *port, bool capable)
> {
> - tcpm_log(port, "Setting usb_comm capable %s", capable ? "true" : "false");
> + tcpm_log(port, "Setting usb_comm capable %s", str_true_false(capable));
>
> if (port->tcpc->set_partner_usb_comm_capable)
> port->tcpc->set_partner_usb_comm_capable(port->tcpc, capable);
>
> --
> 2.43.0
--
heikki
Powered by blists - more mailing lists