[PATCH v3 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[PATCH v3 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b717680377a9..5ed316ea5831 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6439,7 +6439,7 @@ static ssize_t resource_alignment_show(struct bus_type *bus, char *buf)
 
 	spin_lock(&resource_alignment_lock);
 	if (resource_alignment_param)
-		count = scnprintf(buf, PAGE_SIZE, "%s", resource_alignment_param);
+		count = sysfs_emit(buf, "%s", resource_alignment_param);
 	spin_unlock(&resource_alignment_lock);
 
 	/*
-- 
2.31.1

[PATCH v3 02/14] PCI/AER: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/pcie/aer.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ec943cee5ecc..40ef7bed7a77 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -529,21 +529,23 @@ static const char *aer_agent_string[] = {
 		     char *buf)						\
 {									\
 	unsigned int i;							\
-	char *str = buf;						\
 	struct pci_dev *pdev = to_pci_dev(dev);				\
 	u64 *stats = pdev->aer_stats->stats_array;			\
+	size_t len = 0;							\
 									\
 	for (i = 0; i < ARRAY_SIZE(strings_array); i++) {		\
 		if (strings_array[i])					\
-			str += sprintf(str, "%s %llu\n",		\
-				       strings_array[i], stats[i]);	\
+			len += sysfs_emit_at(buf, len, "%s %llu\n",	\
+					     strings_array[i],		\
+					     stats[i]);			\
 		else if (stats[i])					\
-			str += sprintf(str, #stats_array "_bit[%d] %llu\n",\
-				       i, stats[i]);			\
+			len += sysfs_emit_at(buf, len,			\
+					     #stats_array "_bit[%d] %llu\n",\
+					     i, stats[i]);		\
 	}								\
-	str += sprintf(str, "TOTAL_%s %llu\n", total_string,		\
-		       pdev->aer_stats->total_field);			\
-	return str-buf;							\
+	len += sysfs_emit_at(buf, len, "TOTAL_%s %llu\n", total_string,	\
+			     pdev->aer_stats->total_field);		\
+	return len;							\
 }									\
 static DEVICE_ATTR_RO(name)
 
@@ -563,7 +565,7 @@ aer_stats_dev_attr(aer_dev_nonfatal, dev_nonfatal_errs,
 		     char *buf)						\
 {									\
 	struct pci_dev *pdev = to_pci_dev(dev);				\
-	return sprintf(buf, "%llu\n", pdev->aer_stats->field);		\
+	return sysfs_emit(buf, "%llu\n", pdev->aer_stats->field);	\
 }									\
 static DEVICE_ATTR_RO(name)
 
-- 
2.31.1

[PATCH v3 03/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

Modify the function dsm_label_utf16s_to_utf8s() to directly return the
number of bytes written into the buffer so that the strlen() used later
to calculate the length of the buffer can be removed as it would no
longer be needed.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".
  Change style to the preferred one in the drivers/pci/slot.c file.

 drivers/pci/pci-label.c | 18 ++++++++++--------
 drivers/pci/slot.c      | 18 +++++++++---------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
index c32f3b7540e8..000e169c7197 100644
--- a/drivers/pci/pci-label.c
+++ b/drivers/pci/pci-label.c
@@ -139,14 +139,17 @@ enum acpi_attr_enum {
 	ACPI_ATTR_INDEX_SHOW,
 };
 
-static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
+static int dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
 {
 	int len;
+
 	len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
 			      obj->buffer.length,
 			      UTF16_LITTLE_ENDIAN,
 			      buf, PAGE_SIZE);
 	buf[len] = '\n';
+
+	return len;
 }
 
 static int dsm_get_label(struct device *dev, char *buf,
@@ -154,7 +157,7 @@ static int dsm_get_label(struct device *dev, char *buf,
 {
 	acpi_handle handle = ACPI_HANDLE(dev);
 	union acpi_object *obj, *tmp;
-	int len = -1;
+	int len = 0;
 
 	if (!handle)
 		return -1;
@@ -175,20 +178,19 @@ static int dsm_get_label(struct device *dev, char *buf,
 		 * this entry must return a null string.
 		 */
 		if (attr == ACPI_ATTR_INDEX_SHOW) {
-			scnprintf(buf, PAGE_SIZE, "%llu\n", tmp->integer.value);
+			len = sysfs_emit(buf, "%llu\n", tmp->integer.value);
 		} else if (attr == ACPI_ATTR_LABEL_SHOW) {
 			if (tmp[1].type == ACPI_TYPE_STRING)
-				scnprintf(buf, PAGE_SIZE, "%s\n",
-					  tmp[1].string.pointer);
+				len = sysfs_emit(buf, "%s\n",
+						 tmp[1].string.pointer);
 			else if (tmp[1].type == ACPI_TYPE_BUFFER)
-				dsm_label_utf16s_to_utf8s(tmp + 1, buf);
+				len = dsm_label_utf16s_to_utf8s(tmp + 1, buf);
 		}
-		len = strlen(buf) > 0 ? strlen(buf) : -1;
 	}
 
 	ACPI_FREE(obj);
 
-	return len;
+	return len > 0 ? len : -1;
 }
 
 static ssize_t label_show(struct device *dev, struct device_attribute *attr,
diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
index d627dd9179b4..751a26668e3a 100644
--- a/drivers/pci/slot.c
+++ b/drivers/pci/slot.c
@@ -39,19 +39,19 @@ static const struct sysfs_ops pci_slot_sysfs_ops = {
 static ssize_t address_read_file(struct pci_slot *slot, char *buf)
 {
 	if (slot->number == 0xff)
-		return sprintf(buf, "%04x:%02x\n",
-				pci_domain_nr(slot->bus),
-				slot->bus->number);
-	else
-		return sprintf(buf, "%04x:%02x:%02x\n",
-				pci_domain_nr(slot->bus),
-				slot->bus->number,
-				slot->number);
+		return sysfs_emit(buf, "%04x:%02x\n",
+				  pci_domain_nr(slot->bus),
+				  slot->bus->number);
+
+	return sysfs_emit(buf, "%04x:%02x:%02x\n",
+			  pci_domain_nr(slot->bus),
+			  slot->bus->number,
+			  slot->number);
 }
 
 static ssize_t bus_speed_read(enum pci_bus_speed speed, char *buf)
 {
-	return sprintf(buf, "%s\n", pci_speed_string(speed));
+	return sysfs_emit(buf, "%s\n", pci_speed_string(speed));
 }
 
 static ssize_t max_speed_read_file(struct pci_slot *slot, char *buf)
-- 
2.31.1

[PATCH v3 04/14] PCI/MSI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".
  Change style to preferred one as per Joe Perches' suggestion.

 drivers/pci/msi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 217dc9f0231f..9232255c8515 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -464,11 +464,11 @@ static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr,
 		return retval;
 
 	entry = irq_get_msi_desc(irq);
-	if (entry)
-		return sprintf(buf, "%s\n",
-				entry->msi_attrib.is_msix ? "msix" : "msi");
+	if (!entry)
+		return -ENODEV;
 
-	return -ENODEV;
+	return sysfs_emit(buf, "%s\n",
+			  entry->msi_attrib.is_msix ? "msix" : "msi");
 }
 
 static int populate_msi_sysfs(struct pci_dev *pdev)
-- 
2.31.1

[PATCH v3 05/14] PCI/IOV: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/iov.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index afc06e6ce115..a71258347323 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -346,7 +346,7 @@ static ssize_t sriov_totalvfs_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%u\n", pci_sriov_get_totalvfs(pdev));
+	return sysfs_emit(buf, "%u\n", pci_sriov_get_totalvfs(pdev));
 }
 
 static ssize_t sriov_numvfs_show(struct device *dev,
@@ -361,7 +361,7 @@ static ssize_t sriov_numvfs_show(struct device *dev,
 	num_vfs = pdev->sriov->num_VFs;
 	device_unlock(&pdev->dev);
 
-	return sprintf(buf, "%u\n", num_vfs);
+	return sysfs_emit(buf, "%u\n", num_vfs);
 }
 
 /*
@@ -435,7 +435,7 @@ static ssize_t sriov_offset_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%u\n", pdev->sriov->offset);
+	return sysfs_emit(buf, "%u\n", pdev->sriov->offset);
 }
 
 static ssize_t sriov_stride_show(struct device *dev,
@@ -444,7 +444,7 @@ static ssize_t sriov_stride_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%u\n", pdev->sriov->stride);
+	return sysfs_emit(buf, "%u\n", pdev->sriov->stride);
 }
 
 static ssize_t sriov_vf_device_show(struct device *dev,
@@ -453,7 +453,7 @@ static ssize_t sriov_vf_device_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%x\n", pdev->sriov->vf_device);
+	return sysfs_emit(buf, "%x\n", pdev->sriov->vf_device);
 }
 
 static ssize_t sriov_drivers_autoprobe_show(struct device *dev,
@@ -462,7 +462,7 @@ static ssize_t sriov_drivers_autoprobe_show(struct device *dev,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return sprintf(buf, "%u\n", pdev->sriov->drivers_autoprobe);
+	return sysfs_emit(buf, "%u\n", pdev->sriov->drivers_autoprobe);
 }
 
 static ssize_t sriov_drivers_autoprobe_store(struct device *dev,
-- 
2.31.1

[PATCH v3 06/14] PCI/P2PDMA: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/p2pdma.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 196382630363..a1351b3e2c4c 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -53,7 +53,7 @@ static ssize_t size_show(struct device *dev, struct device_attribute *attr,
 	if (pdev->p2pdma->pool)
 		size = gen_pool_size(pdev->p2pdma->pool);
 
-	return scnprintf(buf, PAGE_SIZE, "%zd\n", size);
+	return sysfs_emit(buf, "%zd\n", size);
 }
 static DEVICE_ATTR_RO(size);
 
@@ -66,7 +66,7 @@ static ssize_t available_show(struct device *dev, struct device_attribute *attr,
 	if (pdev->p2pdma->pool)
 		avail = gen_pool_avail(pdev->p2pdma->pool);
 
-	return scnprintf(buf, PAGE_SIZE, "%zd\n", avail);
+	return sysfs_emit(buf, "%zd\n", avail);
 }
 static DEVICE_ATTR_RO(available);
 
@@ -75,8 +75,7 @@ static ssize_t published_show(struct device *dev, struct device_attribute *attr,
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	return scnprintf(buf, PAGE_SIZE, "%d\n",
-			 pdev->p2pdma->p2pmem_published);
+	return sysfs_emit(buf, "%d\n", pdev->p2pdma->p2pmem_published);
 }
 static DEVICE_ATTR_RO(published);
 
-- 
2.31.1

[PATCH v3 07/14] PCI/ASPM: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/pcie/aspm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index ac0557a305af..013a47f587ce 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1208,7 +1208,7 @@ static ssize_t aspm_attr_show_common(struct device *dev,
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
-	return sprintf(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
+	return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
 }
 
 static ssize_t aspm_attr_store_common(struct device *dev,
@@ -1265,7 +1265,7 @@ static ssize_t clkpm_show(struct device *dev,
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
-	return sprintf(buf, "%d\n", link->clkpm_enabled);
+	return sysfs_emit(buf, "%d\n", link->clkpm_enabled);
 }
 
 static ssize_t clkpm_store(struct device *dev,
-- 
2.31.1

[PATCH v3 08/14] PCI: switchtec: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/switch/switchtec.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index ba52459928f7..0b301f8be9ed 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -280,7 +280,7 @@ static ssize_t device_version_show(struct device *dev,
 
 	ver = ioread32(&stdev->mmio_sys_info->device_version);
 
-	return sprintf(buf, "%x\n", ver);
+	return sysfs_emit(buf, "%x\n", ver);
 }
 static DEVICE_ATTR_RO(device_version);
 
@@ -292,7 +292,7 @@ static ssize_t fw_version_show(struct device *dev,
 
 	ver = ioread32(&stdev->mmio_sys_info->firmware_version);
 
-	return sprintf(buf, "%08x\n", ver);
+	return sysfs_emit(buf, "%08x\n", ver);
 }
 static DEVICE_ATTR_RO(fw_version);
 
@@ -344,7 +344,7 @@ static ssize_t component_vendor_show(struct device *dev,
 
 	/* component_vendor field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "none\n");
+		return sysfs_emit(buf, "none\n");
 
 	return io_string_show(buf, &si->gen3.component_vendor,
 			      sizeof(si->gen3.component_vendor));
@@ -359,9 +359,9 @@ static ssize_t component_id_show(struct device *dev,
 
 	/* component_id field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "none\n");
+		return sysfs_emit(buf, "none\n");
 
-	return sprintf(buf, "PM%04X\n", id);
+	return sysfs_emit(buf, "PM%04X\n", id);
 }
 static DEVICE_ATTR_RO(component_id);
 
@@ -373,9 +373,9 @@ static ssize_t component_revision_show(struct device *dev,
 
 	/* component_revision field not supported after gen3 */
 	if (stdev->gen != SWITCHTEC_GEN3)
-		return sprintf(buf, "255\n");
+		return sysfs_emit(buf, "255\n");
 
-	return sprintf(buf, "%d\n", rev);
+	return sysfs_emit(buf, "%d\n", rev);
 }
 static DEVICE_ATTR_RO(component_revision);
 
@@ -384,7 +384,7 @@ static ssize_t partition_show(struct device *dev,
 {
 	struct switchtec_dev *stdev = to_stdev(dev);
 
-	return sprintf(buf, "%d\n", stdev->partition);
+	return sysfs_emit(buf, "%d\n", stdev->partition);
 }
 static DEVICE_ATTR_RO(partition);
 
@@ -393,7 +393,7 @@ static ssize_t partition_count_show(struct device *dev,
 {
 	struct switchtec_dev *stdev = to_stdev(dev);
 
-	return sprintf(buf, "%d\n", stdev->partition_count);
+	return sysfs_emit(buf, "%d\n", stdev->partition_count);
 }
 static DEVICE_ATTR_RO(partition_count);
 
-- 
2.31.1

[PATCH v3 09/14] PCI: rpadlpar: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/hotplug/rpadlpar_sysfs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/rpadlpar_sysfs.c b/drivers/pci/hotplug/rpadlpar_sysfs.c
index dbfa0b55d31a..068b7810a574 100644
--- a/drivers/pci/hotplug/rpadlpar_sysfs.c
+++ b/drivers/pci/hotplug/rpadlpar_sysfs.c
@@ -50,7 +50,7 @@ static ssize_t add_slot_store(struct kobject *kobj, struct kobj_attribute *attr,
 static ssize_t add_slot_show(struct kobject *kobj,
 			     struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "0\n");
+	return sysfs_emit(buf, "0\n");
 }
 
 static ssize_t remove_slot_store(struct kobject *kobj,
@@ -80,7 +80,7 @@ static ssize_t remove_slot_store(struct kobject *kobj,
 static ssize_t remove_slot_show(struct kobject *kobj,
 				struct kobj_attribute *attr, char *buf)
 {
-	return sprintf(buf, "0\n");
+	return sysfs_emit(buf, "0\n");
 }
 
 static struct kobj_attribute add_slot_attr =
-- 
2.31.1

[PATCH v3 10/14] PCI: hotplug: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/hotplug/pci_hotplug_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 5ac31f683b85..058d5937d8a9 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -73,7 +73,7 @@ static ssize_t power_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
@@ -130,7 +130,7 @@ static ssize_t attention_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static ssize_t attention_write_file(struct pci_slot *pci_slot, const char *buf,
@@ -175,7 +175,7 @@ static ssize_t latch_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static struct pci_slot_attribute hotplug_slot_attr_latch = {
@@ -192,7 +192,7 @@ static ssize_t presence_read_file(struct pci_slot *pci_slot, char *buf)
 	if (retval)
 		return retval;
 
-	return sprintf(buf, "%d\n", value);
+	return sysfs_emit(buf, "%d\n", value);
 }
 
 static struct pci_slot_attribute hotplug_slot_attr_presence = {
-- 
2.31.1

[PATCH v3 11/14] PCI: shpchp: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

The sysfs_emit() and sysfs_emit_at() functions were introduced to make
it less ambiguous which function is preferred when writing to the output
buffer in a device attribute's "show" callback [1].

Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
latter is aware of the PAGE_SIZE buffer and correctly returns the number
of bytes written into the buffer.

No functional change intended.

[1] Documentation/filesystems/sysfs.rst

Related to:
  commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/hotplug/shpchp_sysfs.c | 38 +++++++++++++++++-------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/hotplug/shpchp_sysfs.c b/drivers/pci/hotplug/shpchp_sysfs.c
index 45658bb5c554..64beed7a26be 100644
--- a/drivers/pci/hotplug/shpchp_sysfs.c
+++ b/drivers/pci/hotplug/shpchp_sysfs.c
@@ -24,50 +24,54 @@
 static ssize_t show_ctrl(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev;
-	char *out = buf;
 	int index, busnr;
 	struct resource *res;
 	struct pci_bus *bus;
+	size_t len = 0;
 
 	pdev = to_pci_dev(dev);
 	bus = pdev->subordinate;
 
-	out += sprintf(buf, "Free resources: memory\n");
+	len += sysfs_emit_at(buf, len, "Free resources: memory\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_MEM) &&
 				!(res->flags & IORESOURCE_PREFETCH)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: prefetchable memory\n");
+	len += sysfs_emit_at(buf, len, "Free resources: prefetchable memory\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_MEM) &&
 			       (res->flags & IORESOURCE_PREFETCH)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: IO\n");
+	len += sysfs_emit_at(buf, len, "Free resources: IO\n");
 	pci_bus_for_each_resource(bus, res, index) {
 		if (res && (res->flags & IORESOURCE_IO)) {
-			out += sprintf(out, "start = %8.8llx, length = %8.8llx\n",
-				       (unsigned long long)res->start,
-				       (unsigned long long)resource_size(res));
+			len += sysfs_emit_at(buf, len,
+					     "start = %8.8llx, length = %8.8llx\n",
+					     (unsigned long long)res->start,
+					     (unsigned long long)resource_size(res));
 		}
 	}
-	out += sprintf(out, "Free resources: bus numbers\n");
+	len += sysfs_emit_at(buf, len, "Free resources: bus numbers\n");
 	for (busnr = bus->busn_res.start; busnr <= bus->busn_res.end; busnr++) {
 		if (!pci_find_bus(pci_domain_nr(bus), busnr))
 			break;
 	}
 	if (busnr < bus->busn_res.end)
-		out += sprintf(out, "start = %8.8x, length = %8.8x\n",
-				busnr, (int)(bus->busn_res.end - busnr));
+		len += sysfs_emit_at(buf, len,
+				     "start = %8.8x, length = %8.8x\n",
+				     busnr, (int)(bus->busn_res.end - busnr));
 
-	return out - buf;
+	return len;
 }
 static DEVICE_ATTR(ctrl, S_IRUGO, show_ctrl, NULL);
 
-- 
2.31.1

[PATCH v3 12/14] PCI: Fix trailing newline handling of resource_alignment_param

[Krzysztof Wilczyński]

The value of the "resource_alignment" can be specified using a kernel
command-line argument (using the "pci=resource_alignment=") or through
the corresponding sysfs object under the /sys/bus/pci path.

Currently, when the value is set via the kernel command-line argument,
and then subsequently accessed through sysfs object, the value read back
will not be correct, as per:

  # grep -oE 'pci=resource_alignment.+' /proc/cmdline
  pci=resource_alignment=20@00:1f.2
  # cat /sys/bus/pci/resource_alignment
  20@00:1f.

This is also true when the value is set through the sysfs object, but
the trailing newline has not been included, as per:

  # echo -n 20@00:1f.2 > /sys/bus/pci/resource_alignment
  # cat /sys/bus/pci/resource_alignment
  20@00:1f.

When the value set through the sysfs object includes the trailing
newline, then reading it back will work as intended, as per:

  # echo 20@00:1f.2 > /sys/bus/pci/resource_alignment
  # cat /sys/bus/pci/resource_alignment
  20@00:1f.2

To fix this inconsistency, append a trailing newline in the show()
function and strip the trailing line in the store() function if one is
present.

Also, allow for the value previously set using either a command-line
argument or through the sysfs object to be cleared at run-time.

Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs")
Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/pci.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5ed316ea5831..7cde86bdcc8e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6439,34 +6439,37 @@ static ssize_t resource_alignment_show(struct bus_type *bus, char *buf)
 
 	spin_lock(&resource_alignment_lock);
 	if (resource_alignment_param)
-		count = sysfs_emit(buf, "%s", resource_alignment_param);
+		count = sysfs_emit(buf, "%s\n", resource_alignment_param);
 	spin_unlock(&resource_alignment_lock);
 
-	/*
-	 * When set by the command line, resource_alignment_param will not
-	 * have a trailing line feed, which is ugly. So conditionally add
-	 * it here.
-	 */
-	if (count >= 2 && buf[count - 2] != '\n' && count < PAGE_SIZE - 1) {
-		buf[count - 1] = '\n';
-		buf[count++] = 0;
-	}
-
 	return count;
 }
 
 static ssize_t resource_alignment_store(struct bus_type *bus,
 					const char *buf, size_t count)
 {
-	char *param = kstrndup(buf, count, GFP_KERNEL);
+	char *param, *old, *end;
 
+	param = kstrndup(buf, count, GFP_KERNEL);
 	if (!param)
 		return -ENOMEM;
 
+	end = strchr(param, '\n');
+	if (end)
+		*end = '\0';
+
 	spin_lock(&resource_alignment_lock);
-	kfree(resource_alignment_param);
-	resource_alignment_param = param;
+	old = resource_alignment_param;
+	if (strlen(param)) {
+		resource_alignment_param = param;
+	} else {
+		kfree(resource_alignment_param);
+		resource_alignment_param = NULL;
+	}
 	spin_unlock(&resource_alignment_lock);
+
+	kfree(old);
+
 	return count;
 }
 
-- 
2.31.1

[PATCH v3 13/14] PCI/sysfs: Add missing trailing newline to devspec_show()

[Krzysztof Wilczyński]

At the moment, when the value of the "devspec" sysfs object is read from
the user space there will be no newline present, and the utilities such
as the "cat" command won't display the result of the read correctly in
a shell, as the trailing newline is currently missing.

To fix this, append a newline character in the show() function.

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/pci-sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index beb8d1f4fafe..5d63df7c1820 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -537,7 +537,7 @@ static ssize_t devspec_show(struct device *dev,
 
 	if (np == NULL)
 		return 0;
-	return sysfs_emit(buf, "%pOF", np);
+	return sysfs_emit(buf, "%pOF\n", np);
 }
 static DEVICE_ATTR_RO(devspec);
 #endif
-- 
2.31.1

[PATCH v3 14/14] PCI/sysfs: Only show value when driver_override is not NULL

[Krzysztof Wilczyński]

Only expose the value of the "driver_override" variable through the
corresponding sysfs object when a value is actually set.

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
---
Changes in v2:
  None.
Changes in v3:
  Added Logan Gunthorpe's "Reviewed-by".

 drivers/pci/pci-sysfs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5d63df7c1820..4e9f582ca10f 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -580,10 +580,11 @@ static ssize_t driver_override_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-	ssize_t len;
+	ssize_t len = 0;
 
 	device_lock(dev);
-	len = sysfs_emit(buf, "%s\n", pdev->driver_override);
+	if (pdev->driver_override)
+		len = sysfs_emit(buf, "%s\n", pdev->driver_override);
 	device_unlock(dev);
 	return len;
 }
-- 
2.31.1

Re: [PATCH v3 12/14] PCI: Fix trailing newline handling of resource_alignment_param

[Krzysztof Wilczyński]

[+cc Greg and Sasha for visibility]

Hi Bjorn and Logan,

[...]
> Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs")
[...]

This probably would be a candidate for a back-port to stable and
long-term releases.  But, since the move to sysfs_emit()/sysfs_emit_at()
would be then irrelevant, I can split this patch so that it fixes the
issue first, and then other patch will move it to sysfs_emit(), so that
it would be easier to apply when back-porting.

Would this be fine with you Logan?  Especially since you already offered
your review.  I think Bjorn wouldn't mind the split either.

Do let me know folks, and I will change things accordingly.

	Krzysztof

Re: [PATCH v3 12/14] PCI: Fix trailing newline handling of resource_alignment_param

[Logan Gunthorpe]

On 2021-05-21 9:14 a.m., Krzysztof Wilczyński wrote:
> [+cc Greg and Sasha for visibility]
> 
> Hi Bjorn and Logan,
> 
> [...]
>> Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs")
> [...]
> 
> This probably would be a candidate for a back-port to stable and
> long-term releases.  But, since the move to sysfs_emit()/sysfs_emit_at()
> would be then irrelevant, I can split this patch so that it fixes the
> issue first, and then other patch will move it to sysfs_emit(), so that
> it would be easier to apply when back-porting.
> 
> Would this be fine with you Logan?  Especially since you already offered
> your review.  I think Bjorn wouldn't mind the split either.

Totally fine by me. I can re-review the two patches if you like.

Thanks,

Logan

Re: [PATCH v3 12/14] PCI: Fix trailing newline handling of resource_alignment_param

[Greg Kroah-Hartman]

On Fri, May 21, 2021 at 05:14:43PM +0200, Krzysztof Wilczyński wrote:
> [+cc Greg and Sasha for visibility]
> 
> Hi Bjorn and Logan,
> 
> [...]
> > Fixes: e499081da1a2 ("PCI: Force trailing new line to resource_alignment_param in sysfs")
> [...]
> 
> This probably would be a candidate for a back-port to stable and
> long-term releases.  But, since the move to sysfs_emit()/sysfs_emit_at()
> would be then irrelevant, I can split this patch so that it fixes the
> issue first, and then other patch will move it to sysfs_emit(), so that
> it would be easier to apply when back-porting.

sysfs_emit() and sysfs_emit_at() are in all supported stable and
long-term kernel trees at this time, so there's no need to shy away from
using them.

thanks,

greg k-h

Re: [PATCH v3 03/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Bjorn Helgaas]

On Tue, May 18, 2021 at 03:40:58AM +0000, Krzysztof Wilczyński wrote:
> The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> it less ambiguous which function is preferred when writing to the output
> buffer in a device attribute's "show" callback [1].
> 
> Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> latter is aware of the PAGE_SIZE buffer and correctly returns the number
> of bytes written into the buffer.
> 
> Modify the function dsm_label_utf16s_to_utf8s() to directly return the
> number of bytes written into the buffer so that the strlen() used later
> to calculate the length of the buffer can be removed as it would no
> longer be needed.
> 
> No functional change intended.
> 
> [1] Documentation/filesystems/sysfs.rst
> 
> Related to:
>   commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")
> 
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> ---
> Changes in v2:
>   None.
> Changes in v3:
>   Added Logan Gunthorpe's "Reviewed-by".
>   Change style to the preferred one in the drivers/pci/slot.c file.
> 
>  drivers/pci/pci-label.c | 18 ++++++++++--------
>  drivers/pci/slot.c      | 18 +++++++++---------
>  2 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
> index c32f3b7540e8..000e169c7197 100644
> --- a/drivers/pci/pci-label.c
> +++ b/drivers/pci/pci-label.c
> @@ -139,14 +139,17 @@ enum acpi_attr_enum {
>  	ACPI_ATTR_INDEX_SHOW,
>  };
>  
> -static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
> +static int dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
>  {
>  	int len;
> +
>  	len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
>  			      obj->buffer.length,
>  			      UTF16_LITTLE_ENDIAN,
>  			      buf, PAGE_SIZE);
>  	buf[len] = '\n';
> +
> +	return len;
>  }
>  
>  static int dsm_get_label(struct device *dev, char *buf,
> @@ -154,7 +157,7 @@ static int dsm_get_label(struct device *dev, char *buf,
>  {
>  	acpi_handle handle = ACPI_HANDLE(dev);
>  	union acpi_object *obj, *tmp;
> -	int len = -1;
> +	int len = 0;
>  
>  	if (!handle)
>  		return -1;
> @@ -175,20 +178,19 @@ static int dsm_get_label(struct device *dev, char *buf,
>  		 * this entry must return a null string.
>  		 */
>  		if (attr == ACPI_ATTR_INDEX_SHOW) {
> -			scnprintf(buf, PAGE_SIZE, "%llu\n", tmp->integer.value);
> +			len = sysfs_emit(buf, "%llu\n", tmp->integer.value);
>  		} else if (attr == ACPI_ATTR_LABEL_SHOW) {
>  			if (tmp[1].type == ACPI_TYPE_STRING)
> -				scnprintf(buf, PAGE_SIZE, "%s\n",
> -					  tmp[1].string.pointer);
> +				len = sysfs_emit(buf, "%s\n",
> +						 tmp[1].string.pointer);
>  			else if (tmp[1].type == ACPI_TYPE_BUFFER)
> -				dsm_label_utf16s_to_utf8s(tmp + 1, buf);
> +				len = dsm_label_utf16s_to_utf8s(tmp + 1, buf);
>  		}
> -		len = strlen(buf) > 0 ? strlen(buf) : -1;
>  	}
>  
>  	ACPI_FREE(obj);
>  
> -	return len;
> +	return len > 0 ? len : -1;

I really like this, but I would like it even better if the
sysfs_emit() change were easier to review.

It seems pointless that the current code uses strlen() when
scnprintf() and dsm_label_utf16s_to_utf8s() both have that
information and we just throw it away.

I think it should be possible to split the len and
dsm_label_utf16s_to_utf8s() changes to a separate patch, which would
remove the need for the strlen, and then the conversion to
sysfs_emit() would be completely trivial like all the rest of them.

My goal is to make all the sysfs_emit() changes look almost
mechanical, with the non-trivial parts separated out.

>  }
>  
>  static ssize_t label_show(struct device *dev, struct device_attribute *attr,
> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c
> index d627dd9179b4..751a26668e3a 100644
> --- a/drivers/pci/slot.c
> +++ b/drivers/pci/slot.c
> @@ -39,19 +39,19 @@ static const struct sysfs_ops pci_slot_sysfs_ops = {
>  static ssize_t address_read_file(struct pci_slot *slot, char *buf)
>  {
>  	if (slot->number == 0xff)
> -		return sprintf(buf, "%04x:%02x\n",
> -				pci_domain_nr(slot->bus),
> -				slot->bus->number);
> -	else
> -		return sprintf(buf, "%04x:%02x:%02x\n",
> -				pci_domain_nr(slot->bus),
> -				slot->bus->number,
> -				slot->number);
> +		return sysfs_emit(buf, "%04x:%02x\n",
> +				  pci_domain_nr(slot->bus),
> +				  slot->bus->number);
> +
> +	return sysfs_emit(buf, "%04x:%02x:%02x\n",
> +			  pci_domain_nr(slot->bus),
> +			  slot->bus->number,
> +			  slot->number);
>  }
>  
>  static ssize_t bus_speed_read(enum pci_bus_speed speed, char *buf)
>  {
> -	return sprintf(buf, "%s\n", pci_speed_string(speed));
> +	return sysfs_emit(buf, "%s\n", pci_speed_string(speed));
>  }
>  
>  static ssize_t max_speed_read_file(struct pci_slot *slot, char *buf)
> -- 
> 2.31.1
> 

Re: [PATCH v3 03/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Bjorn Helgaas]

On Mon, May 24, 2021 at 04:14:32PM -0500, Bjorn Helgaas wrote:
> On Tue, May 18, 2021 at 03:40:58AM +0000, Krzysztof Wilczyński wrote:
> > The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> > it less ambiguous which function is preferred when writing to the output
> > buffer in a device attribute's "show" callback [1].
> > 
> > Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> > and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> > latter is aware of the PAGE_SIZE buffer and correctly returns the number
> > of bytes written into the buffer.
> > 
> > Modify the function dsm_label_utf16s_to_utf8s() to directly return the
> > number of bytes written into the buffer so that the strlen() used later
> > to calculate the length of the buffer can be removed as it would no
> > longer be needed.
> > 
> > No functional change intended.
> > 
> > [1] Documentation/filesystems/sysfs.rst
> > 
> > Related to:
> >   commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")
> > 
> > Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> > Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> > ---
> > Changes in v2:
> >   None.
> > Changes in v3:
> >   Added Logan Gunthorpe's "Reviewed-by".
> >   Change style to the preferred one in the drivers/pci/slot.c file.
> > 
> >  drivers/pci/pci-label.c | 18 ++++++++++--------
> >  drivers/pci/slot.c      | 18 +++++++++---------
> >  2 files changed, 19 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/pci/pci-label.c b/drivers/pci/pci-label.c
> > index c32f3b7540e8..000e169c7197 100644
> > --- a/drivers/pci/pci-label.c
> > +++ b/drivers/pci/pci-label.c
> > @@ -139,14 +139,17 @@ enum acpi_attr_enum {
> >  	ACPI_ATTR_INDEX_SHOW,
> >  };
> >  
> > -static void dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
> > +static int dsm_label_utf16s_to_utf8s(union acpi_object *obj, char *buf)
> >  {
> >  	int len;
> > +
> >  	len = utf16s_to_utf8s((const wchar_t *)obj->buffer.pointer,
> >  			      obj->buffer.length,
> >  			      UTF16_LITTLE_ENDIAN,
> >  			      buf, PAGE_SIZE);
> >  	buf[len] = '\n';
> > +
> > +	return len;
> >  }
> >  
> >  static int dsm_get_label(struct device *dev, char *buf,
> > @@ -154,7 +157,7 @@ static int dsm_get_label(struct device *dev, char *buf,
> >  {
> >  	acpi_handle handle = ACPI_HANDLE(dev);
> >  	union acpi_object *obj, *tmp;
> > -	int len = -1;
> > +	int len = 0;
> >  
> >  	if (!handle)
> >  		return -1;
> > @@ -175,20 +178,19 @@ static int dsm_get_label(struct device *dev, char *buf,
> >  		 * this entry must return a null string.
> >  		 */
> >  		if (attr == ACPI_ATTR_INDEX_SHOW) {
> > -			scnprintf(buf, PAGE_SIZE, "%llu\n", tmp->integer.value);
> > +			len = sysfs_emit(buf, "%llu\n", tmp->integer.value);
> >  		} else if (attr == ACPI_ATTR_LABEL_SHOW) {
> >  			if (tmp[1].type == ACPI_TYPE_STRING)
> > -				scnprintf(buf, PAGE_SIZE, "%s\n",
> > -					  tmp[1].string.pointer);
> > +				len = sysfs_emit(buf, "%s\n",
> > +						 tmp[1].string.pointer);
> >  			else if (tmp[1].type == ACPI_TYPE_BUFFER)
> > -				dsm_label_utf16s_to_utf8s(tmp + 1, buf);
> > +				len = dsm_label_utf16s_to_utf8s(tmp + 1, buf);
> >  		}
> > -		len = strlen(buf) > 0 ? strlen(buf) : -1;
> >  	}
> >  
> >  	ACPI_FREE(obj);
> >  
> > -	return len;
> > +	return len > 0 ? len : -1;
> 
> I really like this, but I would like it even better if the
> sysfs_emit() change were easier to review.
> 
> It seems pointless that the current code uses strlen() when
> scnprintf() and dsm_label_utf16s_to_utf8s() both have that
> information and we just throw it away.
> 
> I think it should be possible to split the len and
> dsm_label_utf16s_to_utf8s() changes to a separate patch, which would
> remove the need for the strlen, and then the conversion to
> sysfs_emit() would be completely trivial like all the rest of them.
> 
> My goal is to make all the sysfs_emit() changes look almost
> mechanical, with the non-trivial parts separated out.

And BTW, when all the sysfs_emit() changes are trivial like that, I
would probably squash them all into one patch that converts all of
drivers/pci/ at once.

That would still leave a few separate patches:

  - This dsm_label_utf16s_to_utf8s() change
  - The resource_alignment newline change
  - The devspec_show newline change
  - The driver_override change

Re: [PATCH v3 12/14] PCI: Fix trailing newline handling of resource_alignment_param

[Krzysztof Wilczyński]

Hi Greg,

[...]
> > This probably would be a candidate for a back-port to stable and
> > long-term releases.  But, since the move to sysfs_emit()/sysfs_emit_at()
> > would be then irrelevant, I can split this patch so that it fixes the
> > issue first, and then other patch will move it to sysfs_emit(), so that
> > it would be easier to apply when back-porting.
> 
> sysfs_emit() and sysfs_emit_at() are in all supported stable and
> long-term kernel trees at this time, so there's no need to shy away from
> using them.

Excellent!  With some changes requested by Bjorn this will then be
easier to apply to current stable and long-term kernels.

	Krzysztof

Re: [PATCH v3 03/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Krzysztof Wilczyński]

Hi Bjorn,

[...]
> > I really like this, but I would like it even better if the
> > sysfs_emit() change were easier to review.
> > 
> > It seems pointless that the current code uses strlen() when
> > scnprintf() and dsm_label_utf16s_to_utf8s() both have that
> > information and we just throw it away.
> > 
> > I think it should be possible to split the len and
> > dsm_label_utf16s_to_utf8s() changes to a separate patch, which would
> > remove the need for the strlen, and then the conversion to
> > sysfs_emit() would be completely trivial like all the rest of them.
> > 
> > My goal is to make all the sysfs_emit() changes look almost
> > mechanical, with the non-trivial parts separated out.
> 
> And BTW, when all the sysfs_emit() changes are trivial like that, I
> would probably squash them all into one patch that converts all of
> drivers/pci/ at once.
> 
> That would still leave a few separate patches:
> 
>   - This dsm_label_utf16s_to_utf8s() change
>   - The resource_alignment newline change
>   - The devspec_show newline change
>   - The driver_override change

Got it!  I will send v4 updated as per the above suggestion.  Also, if
Logan does not mind, I will carry his "Reviewed-by" over as there will
be no changes to the actual code, just how the patch will be arranged.

	Krzysztof

Re: [PATCH v3 03/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions

[Logan Gunthorpe]

On 2021-05-25 4:32 a.m., Krzysztof Wilczyński wrote:
> Hi Bjorn,
> 
> [...]
>>> I really like this, but I would like it even better if the
>>> sysfs_emit() change were easier to review.
>>>
>>> It seems pointless that the current code uses strlen() when
>>> scnprintf() and dsm_label_utf16s_to_utf8s() both have that
>>> information and we just throw it away.
>>>
>>> I think it should be possible to split the len and
>>> dsm_label_utf16s_to_utf8s() changes to a separate patch, which would
>>> remove the need for the strlen, and then the conversion to
>>> sysfs_emit() would be completely trivial like all the rest of them.
>>>
>>> My goal is to make all the sysfs_emit() changes look almost
>>> mechanical, with the non-trivial parts separated out.
>>
>> And BTW, when all the sysfs_emit() changes are trivial like that, I
>> would probably squash them all into one patch that converts all of
>> drivers/pci/ at once.
>>
>> That would still leave a few separate patches:
>>
>>   - This dsm_label_utf16s_to_utf8s() change
>>   - The resource_alignment newline change
>>   - The devspec_show newline change
>>   - The driver_override change
> 
> Got it!  I will send v4 updated as per the above suggestion.  Also, if
> Logan does not mind, I will carry his "Reviewed-by" over as there will
> be no changes to the actual code, just how the patch will be arranged.

Yup, I'm good with that. Still looks fine by me.

Logan