From: Jeff LaBundy <jeff@labundy.com>
To: ye.xingchen@zte.com.cn
Cc: nick@shmanahar.org, dmitry.torokhov@gmail.com,
giulio.benetti@benettiengineering.com,
dario.binacchi@amarulasolutions.com,
michael@amarulasolutions.com, oliver.graute@kococonnector.com,
wsa+renesas@sang-engineering.com, u.kleine-koenig@pengutronix.de,
johan@kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] Input: touchscreen: use sysfs_emit() to instead of scnprintf()
Date: Mon, 5 Dec 2022 20:49:49 -0600 [thread overview]
Message-ID: <Y46tzRGA/sbkzx7N@nixie71> (raw)
In-Reply-To: <202212021104265067026@zte.com.cn>
Hi Ye,
On Fri, Dec 02, 2022 at 11:04:26AM +0800, ye.xingchen@zte.com.cn wrote:
> From: ye xingchen <ye.xingchen@zte.com.cn>
>
> Replace the open-code with sysfs_emit() to simplify the code.
>
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn>
> ---
> v2 -> v3
> Fix the code style.
NAK, this is the opposite of what I suggested. Before, you had this:
return sysfs_emit(buf, "%u.%u.%u.%u:%u.%u\n",
be16_to_cpu(iqs5xx->dev_id_info.prod_num),
Now, you have this (worse):
return sysfs_emit(buf, "%u.%u.%u.%u:%u.%u\n",
be16_to_cpu(iqs5xx->dev_id_info.prod_num),
We went from one column off to two columns off. We want:
return sysfs_emit(buf, "%u.%u.%u.%u:%u.%u\n",
be16_to_cpu(iqs5xx->dev_id_info.prod_num),
Please maintain the vertical alignment that was previously set forth
in these drivers.
> drivers/input/touchscreen/atmel_mxt_ts.c | 7 +++----
> drivers/input/touchscreen/edt-ft5x06.c | 2 +-
> drivers/input/touchscreen/hideep.c | 6 ++----
> drivers/input/touchscreen/hycon-hy46xx.c | 2 +-
> drivers/input/touchscreen/ilitek_ts_i2c.c | 15 +++++++--------
> drivers/input/touchscreen/iqs5xx.c | 12 ++++++------
> drivers/input/touchscreen/usbtouchscreen.c | 4 ++--
> drivers/input/touchscreen/wdt87xx_i2c.c | 6 +++---
> 8 files changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index ccecd1441f0b..ea43caf20791 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -2761,8 +2761,8 @@ static ssize_t mxt_fw_version_show(struct device *dev,
> {
> struct mxt_data *data = dev_get_drvdata(dev);
> struct mxt_info *info = data->info;
> - return scnprintf(buf, PAGE_SIZE, "%u.%u.%02X\n",
> - info->version >> 4, info->version & 0xf, info->build);
> + return sysfs_emit(buf, "%u.%u.%02X\n",
> + info->version >> 4, info->version & 0xf, info->build);
> }
>
> /* Hardware Version is returned as FamilyID.VariantID */
> @@ -2771,8 +2771,7 @@ static ssize_t mxt_hw_version_show(struct device *dev,
> {
> struct mxt_data *data = dev_get_drvdata(dev);
> struct mxt_info *info = data->info;
> - return scnprintf(buf, PAGE_SIZE, "%u.%u\n",
> - info->family_id, info->variant_id);
> + return sysfs_emit(buf, "%u.%u\n", info->family_id, info->variant_id);
> }
>
> static ssize_t mxt_show_instance(char *buf, int count,
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index 9ac1378610bc..b2ec2e04f943 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -445,7 +445,7 @@ static ssize_t edt_ft5x06_setting_show(struct device *dev,
> *field = val;
> }
>
> - count = scnprintf(buf, PAGE_SIZE, "%d\n", val);
> + count = sysfs_emit(buf, "%d\n", val);
> out:
> mutex_unlock(&tsdata->mutex);
> return error ?: count;
> diff --git a/drivers/input/touchscreen/hideep.c b/drivers/input/touchscreen/hideep.c
> index e9547ee29756..a9f8ca923586 100644
> --- a/drivers/input/touchscreen/hideep.c
> +++ b/drivers/input/touchscreen/hideep.c
> @@ -922,8 +922,7 @@ static ssize_t hideep_fw_version_show(struct device *dev,
> ssize_t len;
>
> mutex_lock(&ts->dev_mutex);
> - len = scnprintf(buf, PAGE_SIZE, "%04x\n",
> - be16_to_cpu(ts->dwz_info.release_ver));
> + len = sysfs_emit(buf, "%04x\n", be16_to_cpu(ts->dwz_info.release_ver));
> mutex_unlock(&ts->dev_mutex);
>
> return len;
> @@ -937,8 +936,7 @@ static ssize_t hideep_product_id_show(struct device *dev,
> ssize_t len;
>
> mutex_lock(&ts->dev_mutex);
> - len = scnprintf(buf, PAGE_SIZE, "%04x\n",
> - be16_to_cpu(ts->dwz_info.product_id));
> + len = sysfs_emit(buf, "%04x\n", be16_to_cpu(ts->dwz_info.product_id));
> mutex_unlock(&ts->dev_mutex);
>
> return len;
> diff --git a/drivers/input/touchscreen/hycon-hy46xx.c b/drivers/input/touchscreen/hycon-hy46xx.c
> index 891d0430083e..2d34959cb510 100644
> --- a/drivers/input/touchscreen/hycon-hy46xx.c
> +++ b/drivers/input/touchscreen/hycon-hy46xx.c
> @@ -202,7 +202,7 @@ static ssize_t hycon_hy46xx_setting_show(struct device *dev,
> *field = val;
> }
>
> - count = scnprintf(buf, PAGE_SIZE, "%d\n", val);
> + count = sysfs_emit(buf, "%d\n", val);
>
> out:
> mutex_unlock(&tsdata->mutex);
> diff --git a/drivers/input/touchscreen/ilitek_ts_i2c.c b/drivers/input/touchscreen/ilitek_ts_i2c.c
> index c5d259c76adc..f4f4fbd5be97 100644
> --- a/drivers/input/touchscreen/ilitek_ts_i2c.c
> +++ b/drivers/input/touchscreen/ilitek_ts_i2c.c
> @@ -512,12 +512,11 @@ static ssize_t firmware_version_show(struct device *dev,
> struct i2c_client *client = to_i2c_client(dev);
> struct ilitek_ts_data *ts = i2c_get_clientdata(client);
>
> - return scnprintf(buf, PAGE_SIZE,
> - "fw version: [%02X%02X.%02X%02X.%02X%02X.%02X%02X]\n",
> - ts->firmware_ver[0], ts->firmware_ver[1],
> - ts->firmware_ver[2], ts->firmware_ver[3],
> - ts->firmware_ver[4], ts->firmware_ver[5],
> - ts->firmware_ver[6], ts->firmware_ver[7]);
> + return sysfs_emit(buf, "fw version: [%02X%02X.%02X%02X.%02X%02X.%02X%02X]\n",
> + ts->firmware_ver[0], ts->firmware_ver[1],
> + ts->firmware_ver[2], ts->firmware_ver[3],
> + ts->firmware_ver[4], ts->firmware_ver[5],
> + ts->firmware_ver[6], ts->firmware_ver[7]);
> }
> static DEVICE_ATTR_RO(firmware_version);
>
> @@ -527,8 +526,8 @@ static ssize_t product_id_show(struct device *dev,
> struct i2c_client *client = to_i2c_client(dev);
> struct ilitek_ts_data *ts = i2c_get_clientdata(client);
>
> - return scnprintf(buf, PAGE_SIZE, "product id: [%04X], module: [%s]\n",
> - ts->mcu_ver, ts->product_id);
> + return sysfs_emit(buf, "product id: [%04X], module: [%s]\n",
> + ts->mcu_ver, ts->product_id);
> }
> static DEVICE_ATTR_RO(product_id);
>
> diff --git a/drivers/input/touchscreen/iqs5xx.c b/drivers/input/touchscreen/iqs5xx.c
> index 34c4cca57d13..7ace2aacdd5b 100644
> --- a/drivers/input/touchscreen/iqs5xx.c
> +++ b/drivers/input/touchscreen/iqs5xx.c
> @@ -943,12 +943,12 @@ static ssize_t fw_info_show(struct device *dev,
> if (!iqs5xx->dev_id_info.bl_status)
> return -ENODATA;
>
> - return scnprintf(buf, PAGE_SIZE, "%u.%u.%u.%u:%u.%u\n",
> - be16_to_cpu(iqs5xx->dev_id_info.prod_num),
> - be16_to_cpu(iqs5xx->dev_id_info.proj_num),
> - iqs5xx->dev_id_info.major_ver,
> - iqs5xx->dev_id_info.minor_ver,
> - iqs5xx->exp_file[0], iqs5xx->exp_file[1]);
> + return sysfs_emit(buf, "%u.%u.%u.%u:%u.%u\n",
> + be16_to_cpu(iqs5xx->dev_id_info.prod_num),
> + be16_to_cpu(iqs5xx->dev_id_info.proj_num),
> + iqs5xx->dev_id_info.major_ver,
> + iqs5xx->dev_id_info.minor_ver,
> + iqs5xx->exp_file[0], iqs5xx->exp_file[1]);
> }
>
> static DEVICE_ATTR_WO(fw_file);
> diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
> index d6d04b9f04fc..fde420e1ee73 100644
> --- a/drivers/input/touchscreen/usbtouchscreen.c
> +++ b/drivers/input/touchscreen/usbtouchscreen.c
> @@ -456,8 +456,8 @@ static ssize_t mtouch_firmware_rev_show(struct device *dev,
> struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
> struct mtouch_priv *priv = usbtouch->priv;
>
> - return scnprintf(output, PAGE_SIZE, "%1x.%1x\n",
> - priv->fw_rev_major, priv->fw_rev_minor);
> + return sysfs_emit(output, "%1x.%1x\n",
> + priv->fw_rev_major, priv->fw_rev_minor);
> }
> static DEVICE_ATTR(firmware_rev, 0444, mtouch_firmware_rev_show, NULL);
>
> diff --git a/drivers/input/touchscreen/wdt87xx_i2c.c b/drivers/input/touchscreen/wdt87xx_i2c.c
> index 166edeb77776..8f1b45ec2618 100644
> --- a/drivers/input/touchscreen/wdt87xx_i2c.c
> +++ b/drivers/input/touchscreen/wdt87xx_i2c.c
> @@ -887,7 +887,7 @@ static ssize_t config_csum_show(struct device *dev,
> cfg_csum = wdt->param.xmls_id1;
> cfg_csum = (cfg_csum << 16) | wdt->param.xmls_id2;
>
> - return scnprintf(buf, PAGE_SIZE, "%x\n", cfg_csum);
> + return sysfs_emit(buf, "%x\n", cfg_csum);
> }
>
> static ssize_t fw_version_show(struct device *dev,
> @@ -896,7 +896,7 @@ static ssize_t fw_version_show(struct device *dev,
> struct i2c_client *client = to_i2c_client(dev);
> struct wdt87xx_data *wdt = i2c_get_clientdata(client);
>
> - return scnprintf(buf, PAGE_SIZE, "%x\n", wdt->param.fw_id);
> + return sysfs_emit(buf, "%x\n", wdt->param.fw_id);
> }
>
> static ssize_t plat_id_show(struct device *dev,
> @@ -905,7 +905,7 @@ static ssize_t plat_id_show(struct device *dev,
> struct i2c_client *client = to_i2c_client(dev);
> struct wdt87xx_data *wdt = i2c_get_clientdata(client);
>
> - return scnprintf(buf, PAGE_SIZE, "%x\n", wdt->param.plat_id);
> + return sysfs_emit(buf, "%x\n", wdt->param.plat_id);
> }
>
> static ssize_t update_config_store(struct device *dev,
> --
> 2.25.1
Kind regards,
Jeff LaBundy
prev parent reply other threads:[~2022-12-06 2:49 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <202212021104265067026@zte.com.cn>
2022-12-02 12:09 ` [PATCH v3] Input: touchscreen: use sysfs_emit() to instead of scnprintf() Oliver Graute
2022-12-06 2:49 ` Jeff LaBundy [this message]
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=Y46tzRGA/sbkzx7N@nixie71 \
--to=jeff@labundy.com \
--cc=dario.binacchi@amarulasolutions.com \
--cc=dmitry.torokhov@gmail.com \
--cc=giulio.benetti@benettiengineering.com \
--cc=johan@kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michael@amarulasolutions.com \
--cc=nick@shmanahar.org \
--cc=oliver.graute@kococonnector.com \
--cc=u.kleine-koenig@pengutronix.de \
--cc=wsa+renesas@sang-engineering.com \
--cc=ye.xingchen@zte.com.cn \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).