| 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: <20250114204323.GB29414@pendragon.ideasonboard.com>
Date: Tue, 14 Jan 2025 22:43:23 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Ray Jui <rjui@...adcom.com>, Scott Branden <sbranden@...adcom.com>,
Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
Michal Simek <michal.simek@....com>, linux-phy@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] phy: Use str_enable_disable-like helpers
Hi Krzysztof,
Thank you for the patch.
On Tue, Jan 14, 2025 at 09:35:37PM +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.
As commented in a similar patch, please drop these changes from drivers
I maintain.
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> ---
> drivers/phy/broadcom/phy-bcm-cygnus-pcie.c | 3 ++-
> drivers/phy/realtek/phy-rtk-usb2.c | 15 ++++++++-------
> drivers/phy/realtek/phy-rtk-usb3.c | 9 +++++----
> drivers/phy/ti/phy-twl4030-usb.c | 4 ++--
> drivers/phy/xilinx/phy-zynqmp.c | 3 ++-
> 5 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/phy/broadcom/phy-bcm-cygnus-pcie.c b/drivers/phy/broadcom/phy-bcm-cygnus-pcie.c
> index 462c61a24ec5..562d7d7151c5 100644
> --- a/drivers/phy/broadcom/phy-bcm-cygnus-pcie.c
> +++ b/drivers/phy/broadcom/phy-bcm-cygnus-pcie.c
> @@ -7,6 +7,7 @@
> #include <linux/of.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> +#include <linux/string_choices.h>
>
> #define PCIE_CFG_OFFSET 0x00
> #define PCIE1_PHY_IDDQ_SHIFT 10
> @@ -86,7 +87,7 @@ static int cygnus_pcie_power_config(struct cygnus_pcie_phy *phy, bool enable)
>
> mutex_unlock(&core->lock);
> dev_dbg(core->dev, "PCIe PHY %d %s\n", phy->id,
> - enable ? "enabled" : "disabled");
> + str_enabled_disabled(enable));
> return 0;
> }
>
> diff --git a/drivers/phy/realtek/phy-rtk-usb2.c b/drivers/phy/realtek/phy-rtk-usb2.c
> index 248550ef98ca..14543636796d 100644
> --- a/drivers/phy/realtek/phy-rtk-usb2.c
> +++ b/drivers/phy/realtek/phy-rtk-usb2.c
> @@ -14,6 +14,7 @@
> #include <linux/debugfs.h>
> #include <linux/nvmem-consumer.h>
> #include <linux/regmap.h>
> +#include <linux/string_choices.h>
> #include <linux/sys_soc.h>
> #include <linux/mfd/syscon.h>
> #include <linux/phy/phy.h>
> @@ -729,7 +730,7 @@ static int rtk_usb2_parameter_show(struct seq_file *s, void *unused)
>
> seq_puts(s, "Property:\n");
> seq_printf(s, " check_efuse: %s\n",
> - phy_cfg->check_efuse ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->check_efuse));
> seq_printf(s, " check_efuse_version: %d\n",
> phy_cfg->check_efuse_version);
> seq_printf(s, " efuse_dc_driving_rate: %d\n",
> @@ -741,17 +742,17 @@ static int rtk_usb2_parameter_show(struct seq_file *s, void *unused)
> seq_printf(s, " dc_disconnect_mask: 0x%x\n",
> phy_cfg->dc_disconnect_mask);
> seq_printf(s, " usb_dc_disconnect_at_page0: %s\n",
> - phy_cfg->usb_dc_disconnect_at_page0 ? "true" : "false");
> + str_true_false(phy_cfg->usb_dc_disconnect_at_page0));
> seq_printf(s, " do_toggle: %s\n",
> - phy_cfg->do_toggle ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->do_toggle));
> seq_printf(s, " do_toggle_driving: %s\n",
> - phy_cfg->do_toggle_driving ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->do_toggle_driving));
> seq_printf(s, " driving_updated_for_dev_dis: 0x%x\n",
> phy_cfg->driving_updated_for_dev_dis);
> seq_printf(s, " use_default_parameter: %s\n",
> - phy_cfg->use_default_parameter ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->use_default_parameter));
> seq_printf(s, " is_double_sensitivity_mode: %s\n",
> - phy_cfg->is_double_sensitivity_mode ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->is_double_sensitivity_mode));
>
> for (index = 0; index < rtk_phy->num_phy; index++) {
> struct phy_parameter *phy_parameter;
> @@ -830,7 +831,7 @@ static int rtk_usb2_parameter_show(struct seq_file *s, void *unused)
> seq_printf(s, " efuse_usb_dc_dis: %d\n",
> (int)phy_parameter->efuse_usb_dc_dis);
> seq_printf(s, " inverse_hstx_sync_clock: %s\n",
> - phy_parameter->inverse_hstx_sync_clock ? "Enable" : "Disable");
> + str_enable_disable(phy_parameter->inverse_hstx_sync_clock));
> seq_printf(s, " driving_level: %d\n",
> phy_parameter->driving_level);
> seq_printf(s, " driving_level_compensate: %d\n",
> diff --git a/drivers/phy/realtek/phy-rtk-usb3.c b/drivers/phy/realtek/phy-rtk-usb3.c
> index cce453686db2..77ce58f98302 100644
> --- a/drivers/phy/realtek/phy-rtk-usb3.c
> +++ b/drivers/phy/realtek/phy-rtk-usb3.c
> @@ -14,6 +14,7 @@
> #include <linux/debugfs.h>
> #include <linux/nvmem-consumer.h>
> #include <linux/regmap.h>
> +#include <linux/string_choices.h>
> #include <linux/sys_soc.h>
> #include <linux/mfd/syscon.h>
> #include <linux/phy/phy.h>
> @@ -363,13 +364,13 @@ static int rtk_usb3_parameter_show(struct seq_file *s, void *unused)
>
> seq_puts(s, "Property:\n");
> seq_printf(s, " check_efuse: %s\n",
> - phy_cfg->check_efuse ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->check_efuse));
> seq_printf(s, " do_toggle: %s\n",
> - phy_cfg->do_toggle ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->do_toggle));
> seq_printf(s, " do_toggle_once: %s\n",
> - phy_cfg->do_toggle_once ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->do_toggle_once));
> seq_printf(s, " use_default_parameter: %s\n",
> - phy_cfg->use_default_parameter ? "Enable" : "Disable");
> + str_enable_disable(phy_cfg->use_default_parameter));
>
> for (index = 0; index < rtk_phy->num_phy; index++) {
> struct phy_reg *phy_reg;
> diff --git a/drivers/phy/ti/phy-twl4030-usb.c b/drivers/phy/ti/phy-twl4030-usb.c
> index 6f12b38cd894..4bf4c4f93b4c 100644
> --- a/drivers/phy/ti/phy-twl4030-usb.c
> +++ b/drivers/phy/ti/phy-twl4030-usb.c
> @@ -26,6 +26,7 @@
> #include <linux/usb/ulpi.h>
> #include <linux/mfd/twl.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/string_choices.h>
> #include <linux/err.h>
> #include <linux/slab.h>
>
> @@ -567,8 +568,7 @@ static ssize_t vbus_show(struct device *dev,
> int ret = -EINVAL;
>
> mutex_lock(&twl->lock);
> - ret = sprintf(buf, "%s\n",
> - twl->vbus_supplied ? "on" : "off");
> + ret = sprintf(buf, "%s\n", str_on_off(twl->vbus_supplied));
> mutex_unlock(&twl->lock);
>
> return ret;
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index 05a4a59f7c40..b96bdc39bcc7 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -23,6 +23,7 @@
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/slab.h>
> +#include <linux/string_choices.h>
>
> #include <dt-bindings/phy/phy.h>
>
> @@ -865,7 +866,7 @@ static int xpsgtr_status_read(struct seq_file *seq, void *data)
> seq_printf(seq, "Reference clock: %u (%pC)\n", gtr_phy->refclk, clk);
> seq_printf(seq, "Reference rate: %lu\n", clk_get_rate(clk));
> seq_printf(seq, "PLL locked: %s\n",
> - pll_status & PLL_STATUS_LOCKED ? "yes" : "no");
> + str_yes_no(pll_status & PLL_STATUS_LOCKED));
>
> mutex_unlock(>r_phy->phy->mutex);
> return 0;
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists