[+cc Sebastian]
On Thu, Jun 03, 2021 at 12:01:10AM +0000, Krzysztof Wilczyński wrote:
> 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.
There are a few other devspec_show() functions:
arch/powerpc/platforms/pseries/ibmebus.c
arch/powerpc/platforms/pseries/vio.c
arch/sparc/kernel/vio.c
drivers/usb/core/sysfs.c
and they all include the newline. So I assume it's not likely that
this minor user-visible change will break something.
I did cc: Sebastian, since his dfc73e7acd99 ("PCI: Move Open Firmware
devspec attribute to PCI common code") moved this code to pci-sysfs.c
in the first place (it came from microblaze and powerpc code that
*also* did not include the newline).
I notice that pci-sysfs.c is the only one that returns 0 if of_node is
NULL. Probably makes more sense than what the others do. I'm
guessing the others would print "(null)" which doesn't seem quite
right for a sysfs attribute.
But pci-sysfs.c also goes to a little more work than necessary to look
up of_node:
struct pci_dev *pdev = to_pci_dev(dev);
struct device_node *np = pci_device_to_OF_node(pdev);
when I think this would be equivalent:
struct device_node *np = dev->of_node;
Some of the others do a similar dance with struct platform_device.
Why'd I write all the above? I dunno; this looks good to me, no
action required for you :)
> Signed-off-by: Krzysztof Wilczyński <kw@xxxxxxxxx>
> Reviewed-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
> ---
> 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
>
