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 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.