Re: [PATCH 0/3] PCI: dwc: Improve code readability
Hello, [...] > > > I confirmed that the uintptr_t fixed the issue. > > > > We declined a similar fix in the past[1] ... > > > > > I also think that adding a new struct with the mode is overkill. > > > > ... with the hopes that a driver could drop the switch statements in place > > of using the other pattern. Also, to be consistent with other drivers that > > do this already. > > Note that the issue of casting is something we cannot fix easily: > some *_device_id structs use "kernel_ulong_t" for the "data" member, > others use "void *". > > git grep -W "_device_id" -- include/linux/mod_devicetable.h | grep data > > In addition, several drivers use multiple types of device IDs, so you > cannot settle on one type to avoid casts. > > Also, putting enum values in instances of that struct, as suggested, > increases kernel size, for IMHO no additional gain. All good points! Thank you for taking the time to get back to me. Appreciated. :) > If there is more data to put in the struct, I agree it makes sense to use > a struct. Yeah. Perhaps if there is such a need in the future, indeed. Krzysztof
Re: [PATCH 0/3] PCI: dwc: Improve code readability
Hello, [...] > > > Now, while you are looking at things, can you also take care about the > > > following: > > > > > > drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to > > > smaller integer type 'enum dw_pcie_device_mode' > > from 'const void *' [-Wvoid-pointer-to-enum-cast] > > Thank you for the report! > > > > This requires adding structs for each data member of the of_device_id > > > type. > > > > That sounds like overkill to me. > > An intermediate cast to uintptr_t should fix the issue as well. > > I confirmed that the uintptr_t fixed the issue. We declined a similar fix in the past[1] ... > I also think that adding a new struct with the mode is overkill. ... with the hopes that a driver could drop the switch statements in place of using the other pattern. Also, to be consistent with other drivers that do this already. > So, I would like to fix the issue by using the cast of uintptr_t. Sure. I appreciate that this would be more work. When you send your patch, can you include an update to the iproc driver (and credit the original author from [1])? I would appreciate it. 1. https://lore.kernel.org/linux-pci/20230814230008.GA196797@bhelgaas/ Krzysztof
Re: [PATCH 0/3] PCI: dwc: Improve code readability
Hi Yoshihiro! > This patch series is based on the latest pci.git / next branch. [...] Thank you for following up to tidy things up! Much appreciated. Now, while you are looking at things, can you also take care about the following: drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to smaller integer type 'enum dw_pcie_device_mode' from 'const void *' [-Wvoid-pointer-to-enum-cast] This requires adding structs for each data member of the of_device_id type. Some examples from other drivers: - https://elixir.bootlin.com/linux/v6.6.1/source/drivers/pci/controller/dwc/pcie-tegra194.c#L2440 - https://elixir.bootlin.com/linux/v6.6.1/source/drivers/pci/controller/dwc/pci-keystone.c#L1074 Thank you! :) Krzysztof
Re: [PATCH 00/24 v2] Documentation: correct lots of spelling errors (series 1)
Hello, > Correct many spelling errors in Documentation/ as reported by codespell. > > Maintainers of specific kernel subsystems are only Cc-ed on their > respective patches, not the entire series. > > These patches are based on linux-next-20230209. > [...] > [PATCH 13/24] Documentation: PCI: correct spelling [...] Applied to misc, thank you! [1/1] Documentation: PCI: correct spelling https://git.kernel.org/pci/pci/c/b58d6d89ae02 Krzysztof
Re: [PATCH v3 0/4] PCI: Add pci_dev_for_each_resource() helper and
Hello, > Provide two new helper macros to iterate over PCI device resources and > convert users. > > Looking at it, refactor existing pci_bus_for_each_resource() and convert > users accordingly. > > This applies on top of this patch Mika sent out earlier: > https://lore.kernel.org/r/20221114115953.40236-1-mika.westerb...@linux.intel.com > > Changelog v3: > - rebased on top of v2 by Mika, see above > - added tag to pcmcia patch (Dominik) [...] Thank you Andy for all the improvements and Mika for the idea! For the whole series: Reviewed-by: Krzysztof Wilczyński Looks very nice! Krzysztof
Re: [PATCH v2 4/4] pcmcia: Convert to use pci_bus_for_each_resource_p()
Hello,
[...]
> > > -
> > > - for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) {
> > > - res = s->cb_dev->bus->resource[i];
> > > -#else
> > > - pci_bus_for_each_resource(s->cb_dev->bus, res, i) {
> > > #endif
> > > +
> > > + pci_bus_for_each_resource_p(s->cb_dev->bus, res) {
> > > if (!res)
> > > continue;
> >
> > Doesn't this remove the proper iterator for X86? Even if that is the right
> > thing to do, it needs an explict explanation.
>
> I dunno what was in 2010, but reading code now I have found no differences in
> the logic on how resources are being iterated in these two pieces of code.
This code is over a decade old (13 years old to be precise) and there was
something odd between Bjorn's and Jesse's patches, as per:
89a74ecccd1f ("PCI: add pci_bus_for_each_resource(), remove direct
bus->resource[] refs")
cf26e8dc4194 ("pcmcia: do not autoadd root PCI bus resources")
> But fine, I will add a line to a commit message about this change.
I wouldn't, personally. The change you are proposing is self-explanatory
and somewhat in-line with what is there already - unless I am also reading
the current implementation wrong.
That said, Dominik is the maintainer of PCMCIA driver, so his is the last
word, so to speak. :)
> Considering this is done, can you issue your conditional tag so I will
> incorporate it in v3?
No need, really. Again, unless Dominik thinks otherwise.
Krzysztof
Re: [PATCH v4] PCI/ERR: Use pcie_aer_is_native() to judge whether OS owns AER
Hello Zhuo, > Use pcie_aer_is_native() in place of "host->native_aer || > pcie_ports_native" to judge whether OS has native control of AER > in aer_root_reset() and pcie_do_recovery(). > > Replace "dev->aer_cap && (pcie_ports_native || host->native_aer)" in > get_port_device_capability() with pcie_aer_is_native(), which has no > functional changes. > > Signed-off-by: Zhuo Chen > --- > Changelog: > v4: > - Use pcie_aer_is_native() instead in aer_root_reset(). > v3: > - Simplify why we use pcie_aer_is_native(). > - Revert modification of pci_aer_clear_nonfatal_status() and comments. > v2: > - Add details and note in commit log. [...] A similar change has been proposed in the past, and back then, Bjorn had a few questions and concerns. Have a look at the entire discussion: https://lore.kernel.org/linux-pci/1612490648-44817-1-git-send-email-tanxiao...@huawei.com/ If you think that the proposed changes are fine and can address some of Bjorn's concerns, then it would be great. Thank you in advance! Krzysztof
Re: [PATCH 1/2] sizes.h: Add SZ_1T macro
Hello Christophe, > Today drivers/pci/controller/pci-xgene.c defines SZ_1T > > Move it into linux/sizes.h so that it can be re-used elsewhere. Sounds like a good idea! By the way, there was an earlier version of this patch, did something happened? I think you simply extracted these changes from the other series, correct? > diff --git a/drivers/pci/controller/pci-xgene.c > b/drivers/pci/controller/pci-xgene.c > index 56d0d50338c8..716dcab5ca47 100644 > --- a/drivers/pci/controller/pci-xgene.c > +++ b/drivers/pci/controller/pci-xgene.c > @@ -49,7 +49,6 @@ > #define EN_REG 0x0001 > #define OB_LO_IO 0x0002 > #define XGENE_PCIE_DEVICEID 0xE004 > -#define SZ_1T(SZ_1G*1024ULL) > #define PIPE_PHY_RATE_RD(src)((0xc000 & (u32)(src)) >> 0xe) > > #define XGENE_V1_PCI_EXP_CAP 0x40 > diff --git a/include/linux/sizes.h b/include/linux/sizes.h > index 1ac79bcee2bb..84aa448d8bb3 100644 > --- a/include/linux/sizes.h > +++ b/include/linux/sizes.h > @@ -47,6 +47,8 @@ > #define SZ_8G_AC(0x2, ULL) > #define SZ_16G _AC(0x4, ULL) > #define SZ_32G _AC(0x8, ULL) > + > +#define SZ_1T_AC(0x100, ULL) > #define SZ_64T _AC(0x4000, ULL) > > #endif /* __LINUX_SIZES_H__ */ Thank you! Reviewed-by: Krzysztof Wilczyński Krzysztof
Re: [PASEMI] Nemo board doesn't recognize any ATA disks with the pci-v5.16 updates
[+CC Adding Robert for visibility]
Hi Arnd,
Thank you looking at this! Much appreciated.
> > > You could attach the kernel config there, too, since it didn't make it
> > > to the mailing list (vger may discard them -- see
> > > http://vger.kernel.org/majordomo-info.html).
> >
> > Bjorn and I looked at which commits that went with a recent Pull Request
> > from us might be causing this, but we are a little bit at loss, and were
> > hoping that you could give us a hand in troubleshooting this.
>
> For reference, these are the patches in that branch that touch any
> interesting files,
> as most of the contents are for pci-controller drivers that are not used on
> powerpc at all:
>
> $ git log --no-merges --oneline 512b7931ad05..dda4b381f05d
> arch/powerpc/ drivers/of/ drivers/pci/*.[ch] include/linux/
> acd61ffb2f16 PCI: Add ACS quirk for Pericom PI7C9X2G switches
> 978fd0056e19 PCI: of: Allow matching of an interrupt-map local to a PCI device
> 041284181226 of/irq: Allow matching of an interrupt-map local to an
> interrupt controller
> 0ab8d0f6ae3f irqdomain: Make of_phandle_args_to_fwspec() generally available
> 5ec0a6fcb60e PCI: Do not enable AtomicOps on VFs
> 7a41ae80bdcb PCI: pci-bridge-emul: Fix emulation of W1C bits
> fd1ae23b495b PCI: Prefer 'unsigned int' over bare 'unsigned'
> ff5d3bb6e16d PCI: Remove redundant 'rc' initialization
> 3331325c6347 PCI/VPD: Use pci_read_vpd_any() in pci_vpd_size()
> e1b0d0bb2032 PCI: Re-enable Downstream Port LTR after reset or hotplug
> ac8e3cef588c PCI/sysfs: Explicitly show first MSI IRQ for 'irq'
> 88dee3b0efe4 PCI: Remove unused pci_pool wrappers
> b5f9c644eb1b PCI: Remove struct pci_dev->driver
> 2a4d9408c9e8 PCI: Use to_pci_driver() instead of pci_dev->driver
> 4141127c44a9 powerpc/eeh: Use to_pci_driver() instead of pci_dev->driver
> f9a6c8ad4922 PCI/ERR: Reduce compile time for CONFIG_PCIEAER=n
> 43e85554d4ed xen/pcifront: Use to_pci_driver() instead of pci_dev->driver
> 34ab316d7287 xen/pcifront: Drop pcifront_common_process() tests of pcidev,
> pdrv
> 9f37ab0412eb PCI/switchtec: Add check of event support
> 5a72431ec318 powerpc/eeh: Use dev_driver_string() instead of struct
> pci_dev->driver->name
> ae232f0970ea PCI: Drop pci_device_probe() test of !pci_dev->driver
> 097d9d414433 PCI: Drop pci_device_remove() test of pci_dev->driver
> 8e9028b3790d PCI: Return NULL for to_pci_driver(NULL)
> 357df2fc0066 PCI: Use unsigned to match sscanf("%x") in
> pci_dev_str_match_path()
> bf2928c7a284 PCI/VPD: Add pci_read/write_vpd_any()
> b2105b9f39b5 PCI: Correct misspelled and remove duplicated words
> 7c3855c423b1 PCI: Coalesce host bridge contiguous apertures
> e0f7b1922358 PCI: Use kstrtobool() directly, sans strtobool() wrapper
> 36f354ec7bf9 PCI/sysfs: Return -EINVAL consistently from "store" functions
> 95e83e219d68 PCI/sysfs: Check CAP_SYS_ADMIN before parsing user input
> af9d82626c8f PCI/ACPI: Remove OSC_PCI_SUPPORT_MASKS and OSC_PCI_CONTROL_MASKS
> 9a0a1417d3bb PCI: Tidy comments
> 06dc660e6eb8 PCI: Rename pcibios_add_device() to pcibios_device_add()
> e3f4bd3462f6 PCI: Mark Atheros QCA6174 to avoid bus reset
> 3a19407913e8 PCI/P2PDMA: Apply bus offset correctly in DMA address calculation
>
> Out of these, I agree that most of them seem harmless, these would
> be the ones I'd try to look at more closely, or maybe revert for testing:
>
> 978fd0056e19 PCI: of: Allow matching of an interrupt-map local to a PCI device
> 041284181226 of/irq: Allow matching of an interrupt-map local to an
> interrupt controller
> e1b0d0bb2032 PCI: Re-enable Downstream Port LTR after reset or hotplug
> 7c3855c423b1 PCI: Coalesce host bridge contiguous apertures
> 3a19407913e8 PCI/P2PDMA: Apply bus offset correctly in DMA address calculation
Robert would you be able build a kernel without the patches Arnd singled
out as potential curlprits? Might be expidite some troubleshooting saving
a lot of time doing bisect.
I wonder if this will help you with the following problem:
https://lore.kernel.org/linux-pci/CAP145pjO9zdGgutHP=of0H+L1=nsz097zf73i7zym2-nwuw...@mail.gmail.com/
Krzysztof
Re: [PASEMI] Nemo board doesn't recognize any ATA disks with the pci-v5.16 updates
[+CC Adding Jens and Damien to get their opinion about the problem at hand] Hello Jens and Damien, Sorry to bother both of you, but we are having a problem that most definitely requires someone with an extensive expertise in storage, as per the quoted message from Christian below: > > > The Nemo board [1] doesn't recognize any ATA disks with the pci-v5.16 > > > updates [2]. > > > > > > Error messages: > > > > > > ata4.00: gc timeout cmd 0xec > > > ata4.00: failed to IDENTIFY (I/O error, error_mask=0x4) > > > ata1.00: gc timeout cmd 0xec > > > ata1.00: failed to IDENTIFY (I/O error, error_mask=0x4) > > > ata3.00: gc timeout cmd 0xec > > > ata3.00: failed to IDENTIFY (I/O error, error_mask=0x4) The error message is also not very detailed and we aren't really sure what the issue coming from the PCI sub-system might be causing or leading to this. > > > > > > I was able to revert the new pci-v5.16 updates [2]. After a new compiling, > > > the kernel recognize all ATA disks correctly. > > > > > > Could you please check the pci-v5.16 updates [2]? > > > > > > Please find attached the kernel config. > > > > > > Thanks, > > > Christian > > > > > > [1] https://en.wikipedia.org/wiki/AmigaOne_X1000 > > > [2] > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0c5c62ddf88c34bc83b66e4ac9beb2bb0e1887d4 > > Sorry for the breakage, and thank you very much for the report. Can > you please collect the complete dmesg logs before and after the > pci-v5.16 changes and the "sudo lspci -vv" output from before the > changes? > > You can attach them at https://bugzilla.kernel.org if you don't have > a better place to put them. > > You could attach the kernel config there, too, since it didn't make it > to the mailing list (vger may discard them -- see > http://vger.kernel.org/majordomo-info.html). Bjorn and I looked at which commits that went with a recent Pull Request from us might be causing this, but we are a little bit at loss, and were hoping that you could give us a hand in troubleshooting this. Thank you in advance! Krzysztof
[PATCH 1/2] PCI: Use pcie_reset_state_t type in function arguments
The pcie_reset_state_t type has been introduced in the commit
f7bdd12d234d ("pci: New PCI-E reset API") along with the enum
pcie_reset_state, but it has never been used for anything else
other than to define the members of the enumeration set in the
enum pcie_reset_state.
Thus, replace the direct use of enum pcie_reset_state in function
arguments and replace it with pcie_reset_state_t type so that the
argument type matches the type used in enum pcie_reset_state.
Signed-off-by: Krzysztof Wilczyński
---
drivers/pci/pci.c | 4 ++--
include/linux/pci.h | 5 ++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index aacf575c15cf..5c3386a73eb1 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2194,7 +2194,7 @@ EXPORT_SYMBOL(pci_disable_device);
* implementation. Architecture implementations can override this.
*/
int __weak pcibios_set_pcie_reset_state(struct pci_dev *dev,
- enum pcie_reset_state state)
+ pcie_reset_state_t state)
{
return -EINVAL;
}
@@ -2206,7 +2206,7 @@ int __weak pcibios_set_pcie_reset_state(struct pci_dev
*dev,
*
* Sets the PCI reset state for the device.
*/
-int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
+int pci_set_pcie_reset_state(struct pci_dev *dev, pcie_reset_state_t state)
{
return pcibios_set_pcie_reset_state(dev, state);
}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 540b377ca8f6..15f93de69e6a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -191,7 +191,6 @@ enum {
};
typedef unsigned int __bitwise pcie_reset_state_t;
-
enum pcie_reset_state {
/* Reset is NOT asserted (Use to deassert reset) */
pcie_deassert_reset = (__force pcie_reset_state_t) 1,
@@ -1205,7 +1204,7 @@ extern unsigned int pcibios_max_latency;
void pci_set_master(struct pci_dev *dev);
void pci_clear_master(struct pci_dev *dev);
-int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
+int pci_set_pcie_reset_state(struct pci_dev *dev, pcie_reset_state_t state);
int pci_set_cacheline_size(struct pci_dev *dev);
int __must_check pci_set_mwi(struct pci_dev *dev);
int __must_check pcim_set_mwi(struct pci_dev *dev);
@@ -2079,7 +2078,7 @@ extern u8 pci_cache_line_size;
void pcibios_disable_device(struct pci_dev *dev);
void pcibios_set_master(struct pci_dev *dev);
int pcibios_set_pcie_reset_state(struct pci_dev *dev,
-enum pcie_reset_state state);
+pcie_reset_state_t state);
int pcibios_add_device(struct pci_dev *dev);
void pcibios_release_device(struct pci_dev *dev);
#ifdef CONFIG_PCI
--
2.32.0
[PATCH 2/2] powerpc/eeh: Use pcie_reset_state_t type in function arguments
The pcie_reset_state_t type has been introduced in the commit
f7bdd12d234d ("pci: New PCI-E reset API") along with the enum
pcie_reset_state, but it has never been used for anything else
other than to define the members of the enumeration set in the
enum pcie_reset_state.
Thus, replace the direct use of enum pcie_reset_state in function
arguments and replace it with pcie_reset_state_t type so that the
argument type matches the type used in enum pcie_reset_state.
Signed-off-by: Krzysztof Wilczyński
---
arch/powerpc/kernel/eeh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 3bbdcc86d01b..15485abb89ff 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -714,7 +714,7 @@ static void eeh_restore_dev_state(struct eeh_dev *edev,
void *userdata)
* Return value:
* 0 if success
*/
-int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state
state)
+int pcibios_set_pcie_reset_state(struct pci_dev *dev, pcie_reset_state_t state)
{
struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
struct eeh_pe *pe = eeh_dev_to_pe(edev);
--
2.32.0
Re: [PATCH] Documentation PCI: Fix typo in pci-error-recovery.rst
Hi Wesley, > Replace "It" with "If", since it is a conditional statement. > [...] Nice catch! > -cycle) and then call slot_reset() again. It the device still can't > +cycle) and then call slot_reset() again. If the device still can't [...] Thank you! Reviewed-by: Krzysztof Wilczyński Krzysztof
Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
Hi Logan,
> > 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")
>
> I re-reviewed the whole series. It still looks good to me.
>
> Very nice solution in patch 12 to the new line issue.
>
> Reviewed-by: Logan Gunthorpe
>
> Thanks,
Thank you!
I will send v3 incorporating the style change as per Joe's suggestion
and carry-over your "Reviewed-by", if you don't mind, as it will be
a trivial change.
Krzysztof
Re: [PATCH v2 04/14] PCI/MSI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
Hi Joe, [...] > > if (entry) > > - return sprintf(buf, "%s\n", > > - entry->msi_attrib.is_msix ? "msix" : "msi"); > > + return sysfs_emit(buf, "%s\n", > > + entry->msi_attrib.is_msix ? "msix" : "msi"); > > > > > > return -ENODEV; > > } > > trivia: reversing the test would be more common style > > if (!entry) > return -ENODEV; > > return sysfs_emit(...); > } Excellent point. I will send v3 later that includes this style change. Thank you! Krzysztof
Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
Hi Joe,
[...]
> Ideally, the additional newline check below this would use sysfs_emit_at
>
> drivers/pci/pci.c- /*
> drivers/pci/pci.c: * When set by the command line,
> resource_alignment_param will not
> drivers/pci/pci.c- * have a trailing line feed, which is ugly. So
> conditionally add
> drivers/pci/pci.c- * it here.
> drivers/pci/pci.c- */
> drivers/pci/pci.c- if (count >= 2 && buf[count - 2] != '\n' && count <
> PAGE_SIZE - 1) {
> drivers/pci/pci.c- buf[count - 1] = '\n';
> drivers/pci/pci.c- buf[count++] = 0;
> drivers/pci/pci.c- }
> drivers/pci/pci.c-
> drivers/pci/pci.c- return count;
I found some inconsistencies with adding newline this way, and decided
to change the code slightly, see:
https://lore.kernel.org/linux-pci/20210515052434.1413236-12...@linux.com/
Krzysztof
Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
Hello, [...] > Reviewed-by: Logan Gunthorpe Please disregard this "Reviewed-by" from Logan for this version, as I've forgotten to remove it before sending v2 after pulling patches using b4. Apologies. Krzysztof
[PATCH v2 14/14] PCI/sysfs: Only show value when driver_override is not NULL
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
---
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
[PATCH v2 13/14] PCI/sysfs: Add missing trailing newline to devspec_show()
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 --- 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 v2 12/14] PCI: Fix trailing newline handling of resource_alignment_param
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
---
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(_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(_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(_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(_alignment_lock);
+
+ kfree(old);
+
return count;
}
--
2.31.1
[PATCH v2 11/14] PCI: shpchp: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 v2 10/14] PCI: hotplug: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 v2 09/14] PCI: rpadlpar: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 v2 08/14] PCI: switchtec: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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(>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(>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, >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 v2 07/14] PCI/ASPM: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 v2 06/14] PCI/P2PDMA: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 v2 05/14] PCI/IOV: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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(>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 v2 04/14] PCI/MSI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
drivers/pci/msi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 217dc9f0231f..dbfec59dfe41 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -465,8 +465,8 @@ static ssize_t msi_mode_show(struct device *dev, struct
device_attribute *attr,
entry = irq_get_msi_desc(irq);
if (entry)
- return sprintf(buf, "%s\n",
- entry->msi_attrib.is_msix ? "msix" : "msi");
+ return sysfs_emit(buf, "%s\n",
+ entry->msi_attrib.is_msix ? "msix" : "msi");
return -ENODEV;
}
--
2.31.1
[PATCH v2 03/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
drivers/pci/pci-label.c | 18 ++
drivers/pci/slot.c | 16
2 files changed, 18 insertions(+), 16 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..7487e8f8f13f 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);
+ return sysfs_emit(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:%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 v2 02/14] PCI/AER: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
Reviewed-by: Logan Gunthorpe
---
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(_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(_alignment_lock);
/*
--
2.31.1
Re: [PATCH 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
Hi Logan, [...] > Thanks, this is a great cleanup. I've reviewed the entire series. > > Reviewed-by: Logan Gunthorpe Thank you! Appreciate it! > I agree that the new lines that are missing should be added. While working on the simple change to add the missing new lines, I've found that we have slight inconsistency with handling these in one of the attributes, as per: # uname -r 5.13.0-rc1-00092-gc06a2ba62fc4 # grep -oE 'pci=resource_alignment.+' /proc/cmdline pci=resource_alignment=20@00:1f.2 # cat /sys/bus/pci/resource_alignment 20@00:1f. # echo 20@00:1f.2 > /sys/bus/pci/resource_alignment # cat /sys/bus/pci/resource_alignment 20@00:1f.2 # echo -n 20@00:1f.2 > /sys/bus/pci/resource_alignment # cat /sys/bus/pci/resource_alignment 20@00:1f. I adjusted the code so that we handle the input better in the upcoming v2 - the change is simple, but since it adds more code, I'll drop your "Reviewed-by", so that we can make sure that subsequent review will cover this new change. Sorry for troubles in advance! Krzysztof
Re: [PATCH 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
[+cc Joe for visibility] [...] > spin_lock(_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(_alignment_lock); Following the work that Joe did recently, see: https://lore.kernel.org/lkml/aa1819fa5faf786573df298e5e2e7d357ba7d4ad.ca...@perches.com/ I think we ought to also add the missing newline to our sysfs_emit() and sysfs_emit_at() users, like the one above and the following: drivers/pci/pci-sysfs.c 540: return sysfs_emit(buf, "%pOF", np); To keep things correct and consistent. Bjorn, I can follow-up with a small patch after this one, or send a v2, or, if that would be OK with you, then you could fix it during merging, provided you decide to merge things as-is. Krzysztof
[PATCH 11/11] PCI: shpchp: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 10/11] PCI: hotplug: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 09/11] PCI: rpadlpar: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 08/11] PCI: switchtec: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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(>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(>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, >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 07/11] PCI/ASPM: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 06/11] PCI/P2PDMA: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 05/11] PCI/IOV: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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(>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 04/11] PCI/MSI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
drivers/pci/msi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 217dc9f0231f..dbfec59dfe41 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -465,8 +465,8 @@ static ssize_t msi_mode_show(struct device *dev, struct
device_attribute *attr,
entry = irq_get_msi_desc(irq);
if (entry)
- return sprintf(buf, "%s\n",
- entry->msi_attrib.is_msix ? "msix" : "msi");
+ return sysfs_emit(buf, "%s\n",
+ entry->msi_attrib.is_msix ? "msix" : "msi");
return -ENODEV;
}
--
2.31.1
[PATCH 02/11] PCI/AER: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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 01/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
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(_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(_alignment_lock);
/*
--
2.31.1
[PATCH 03/11] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
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
---
drivers/pci/pci-label.c | 18 ++
drivers/pci/slot.c | 16
2 files changed, 18 insertions(+), 16 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..7487e8f8f13f 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);
+ return sysfs_emit(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:%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] PCI: layerscape: Correct syntax by changing comma to semicolon
Replace command with a semicolon to correct syntax and to prevent potential unspecified behaviour and/or unintended side effects. Related: https://lore.kernel.org/linux-pci/20201216131944.14990-1-zhengyongj...@huawei.com/ Co-authored-by: Zheng Yongjun Signed-off-by: Krzysztof Wilczyński --- drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c index 39fe2ed5a6a2..39f4664bd84c 100644 --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c @@ -154,7 +154,7 @@ static int __init ls_pcie_ep_probe(struct platform_device *pdev) pci->dev = dev; pci->ops = pcie->drvdata->dw_pcie_ops; - ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4), + ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4); pcie->pci = pci; pcie->ls_epc = ls_epc; -- 2.30.1
Re: [PATCH -next] pci/controller/dwc: convert comma to semicolon
Hi, [...] > I would request NXP maintainers to take this patch, rewrite it as > Bjorn requested and resend it as fast as possible, this is a very > relevant fix. [...] Looking at the state of the pci-layerscape-ep.c file in Linus' tree, this still hasn't been fixed, and it has been a while. NXP folks, are you intend to pick this up? Do let us know. Krzysztof
[PATCH v2] PCI: Convert enum pci_bus_flags to bit fields in struct pci_bus
All the flags defined in the enum pci_bus_flags are used to determine
whether a particular feature of a PCI bus is available or not - features
are also often disabled via architecture or device-specific quirk.
These flags are tightly coupled with a PCI buses and PCI bridges and
primarily used in simple binary on/off manner to check whether something
is enabled or disabled, and have almost no other users (with an
exception of the x86 architecture-specific quirk) outside of the PCI
device drivers space.
Therefore, convert enum pci_bus_flags into a set of bit fields in the
struct pci_bus, and then drop said enum and the typedef pci_bus_flags_t.
This will keep PCI device-specific features as part of the struct
pci_dev and make the code that used to use flags simpler.
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
Changes in v2:
Rebased against kernel 5.11.
arch/x86/pci/fixup.c| 6 +++---
drivers/pci/msi.c | 8
drivers/pci/pci-sysfs.c | 14 ++
drivers/pci/pci.c | 2 +-
drivers/pci/pcie/aer.c | 5 ++---
drivers/pci/probe.c | 13 +
drivers/pci/quirks.c| 16
include/linux/pci.h | 20 ++--
8 files changed, 43 insertions(+), 41 deletions(-)
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 0a0e168be1cb..a43316ced8ab 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -641,14 +641,14 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x8c10,
quirk_apple_mbp_poweroff);
* ID, the AER driver should traverse the child device tree, reading
* AER registers to find the faulting device.
*/
-static void quirk_no_aersid(struct pci_dev *pdev)
+static void quirk_no_aer_sid(struct pci_dev *pdev)
{
/* VMD Domain */
if (is_vmd(pdev->bus) && pci_is_root_bus(pdev->bus))
- pdev->bus->bus_flags |= PCI_BUS_FLAGS_NO_AERSID;
+ pdev->bus->no_aer_sid = 1;
}
DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
- PCI_CLASS_BRIDGE_PCI, 8, quirk_no_aersid);
+ PCI_CLASS_BRIDGE_PCI, 8, quirk_no_aer_sid);
static void quirk_intel_th_dnv(struct pci_dev *dev)
{
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 3162f88fe940..134ddf20bde8 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -899,13 +899,13 @@ static int pci_msi_supported(struct pci_dev *dev, int
nvec)
/*
* Any bridge which does NOT route MSI transactions from its
-* secondary bus to its primary bus must set NO_MSI flag on
-* the secondary pci_bus.
+* secondary bus to its primary bus must enable "no_msi" on
+* the secondary bus (pci_bus).
* We expect only arch-specific PCI host bus controller driver
-* or quirks for specific PCI bridges to be setting NO_MSI.
+* or quirks for specific PCI bridges to enable "no_msi".
*/
for (bus = dev->bus; bus; bus = bus->parent)
- if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+ if (bus->no_msi)
return 0;
return 1;
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index fb072f4b3176..414727551660 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -366,9 +366,7 @@ static ssize_t msi_bus_show(struct device *dev, struct
device_attribute *attr,
struct pci_dev *pdev = to_pci_dev(dev);
struct pci_bus *subordinate = pdev->subordinate;
- return sprintf(buf, "%u\n", subordinate ?
- !(subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI)
- : !pdev->no_msi);
+ return sprintf(buf, "%u\n", subordinate ? !subordinate->no_msi :
!pdev->no_msi);
}
static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
@@ -385,9 +383,9 @@ static ssize_t msi_bus_store(struct device *dev, struct
device_attribute *attr,
return -EPERM;
/*
-* "no_msi" and "bus_flags" only affect what happens when a driver
-* requests MSI or MSI-X. They don't affect any drivers that have
-* already requested MSI or MSI-X.
+* "no_msi" enabled for device and bus only affect what happens
+* when a driver requests MSI or MSI-X. They don't affect any
+* drivers that have already requested MSI or MSI-X.
*/
if (!subordinate) {
pdev->no_msi = !val;
@@ -397,9 +395,9 @@ static ssize_t msi_bus_store(struct device *dev, struct
device_attribute *attr,
}
if (val)
- subordinate->bus_flags &= ~PCI_BUS_FLAGS_NO_MSI;
+ subordinate->no_msi = 0;
else
- subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+ subordinate->no_msi =
Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
Hello Nicolas, Florian and Florian,
[...]
> -/* Configuration space read/write support */
> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
> -{
> - return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
> - | ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
> - | (busnr << PCIE_EXT_BUSNUM_SHIFT)
> - | (reg & ~3);
> -}
> -
> static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int
> devfn,
> int where)
> {
> @@ -716,7 +704,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus
> *bus, unsigned int devfn,
> return PCI_SLOT(devfn) ? NULL : base + where;
>
> /* For devices, write to the config space index register */
> - idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
> + idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
> writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
> return base + PCIE_EXT_CFG_DATA + where;
> }
[...]
Passing the hard-coded 0 as the "reg" argument here never actually did
anything, thus the 32 bit alignment was never correctly enforced.
My question would be: should this be 32 bit aligned? It seems like the
intention was to perhaps make the alignment? I am sadly not intimately
familiar with his hardware, so I am not sure if there is something to
fix here or not.
Also, I wonder whether it would be safe to pass the offset (the "where"
variable) rather than hard-coded 0?
Thank you for help in advance!
Bjorn also asked the same question:
https://lore.kernel.org/linux-pci/20201120203428.GA272511@bjorn-Precision-5520/
Krzysztof
Re: [PATCH v5] PCI: Unify ECAM constants in native PCI Express drivers
[+CC David for visibility]
Hi Bjorn,
Thank you for the review!
On 20-11-28 12:35:16, Bjorn Helgaas wrote:
[...]
> It's ironic that we don't use PCIE_ECAM_OFFSET in drivers/pci/ecam.c.
> We could do something like this, which would also let us drop
> .bus_shift completely in all the conforming implementations. It also
> closes the hole that we didn't limit "where" to 4K for
> pci_ecam_map_bus() users.
>
> if (per_bus_mapping) {
> base = cfg->winp[busn];
> busn = 0;
> } else {
> base = cfg->win;
> }
>
> if (cfg->ops->bus_shift) {
> u32 bus_offset = (busn & 0xff) << cfg->ops->bus_shift;
> u32 devfn_offset = (devfn & 0xff) << (cfg->ops->bus_shift - 8);
>
> where &= 0xfff;
>
> return base + (bus_offset | devfn_offset | where);
> }
>
> return base + PCIE_ECAM_OFFSET(busn, devfn, where);
[...]
Thank you for suggesting this! I sent v6 recently that includes this.
> > static void __iomem *ppc4xx_pciex_get_config_base(struct ppc4xx_pciex_port
> > *port,
> > struct pci_bus *bus,
> > - unsigned int devfn)
> > + unsigned int devfn,
> > + int offset)
>
> The interface change (to add "offset") could be a preparatory patch by
> itself.
>
> But I'm actually not sure it's worth even touching this file. This is
> the only place outside drivers/pci that includes linux/pci-ecam.h. I
> think I might rather put PCIE_ECAM_OFFSET() and related things in
> drivers/pci/pci.h and keep it all inside drivers/pci.
Makes sense to drop it. We can always introduce chances on PPC 4xx
platform in the future if we ever want it to leverage all the new macros
and constants.
These changes are not included in v6.
> > static const struct pci_ecam_ops pci_thunder_pem_ops = {
> > - .bus_shift = 24,
> > + .bus_shift = THUNDER_PCIE_ECAM_BUS_SHIFT,
> > .init = thunder_pem_platform_init,
> > .pci_ops= {
> > .map_bus= pci_ecam_map_bus,
>
> This could be split to its own patch, no big deal either way.
Done. v6 is now a series that includes this as a separate patch.
> > const struct pci_ecam_ops xgene_v2_pcie_ecam_ops = {
> > - .bus_shift = 16,
> > .init = xgene_v2_pcie_ecam_init,
> > .pci_ops= {
> > .map_bus= xgene_pcie_map_bus,
>
> Thanks for mentioning this change in the cover letter. It could also
> be split off to a preparatory patch, since it's not related to
> PCIE_ECAM_OFFSET(), which is the main point of this patch.
Done.
> > static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,
> >unsigned int busno,
> > - unsigned int slot,
> > - unsigned int fn,
> > + unsigned int devfn,
>
> This interface change *could* be a separate preparatory patch, too,
> but I'm starting to feel even more OCD than usual :)
Done. It's a separate patch in v6, although I kept it together with the
change to introduce the PCIE_ECAM_OFFSET() macro since I was retiring the
use of PCI_SLOT() and PCI_FUNC() macros.
> > @@ -94,7 +95,7 @@ struct vmd_dev {
> > struct pci_dev *dev;
> >
> > spinlock_t cfg_lock;
> > - char __iomem*cfgbar;
> > + void __iomem*cfgbar;
>
> This type change might be worth pushing to a separate patch since the
> casting issues are not completely trivial.
Done.
The patch included in the series as part of v6 already got a review from
David Laight (thank you!) who suggests that this might not be a good
idea to do, and keeping existing type would be better.
Krzysztof
Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
Hi Lorenzo! On 20-11-30 11:08:58, Lorenzo Pieralisi wrote: [...] > > Refactor pci_ecam_map_bus() function to use newly added constants so > > that limits to the bus, device function and offset (now limited to 4K as > > per the specification) are in place to prevent the defective or > > malicious caller from supplying incorrect configuration offset and thus > > targeting the wrong device when accessing extended configuration space. > > This refactor also allows for the ".bus_shit" initialisers to be dropped > > > Nice typo, I'd fix it while applying it though if you don't mind ;-), > no need to resend it. Oh doh! Apologies. :) > Jokes aside, nice piece of work, thanks for that. > > > when the user is not using a custom value as a default value will be > > used as per the PCI Express Specification. > > > > Suggested-by: Bjorn Helgaas > > Signed-off-by: Krzysztof Wilczyński > > I think Bjorn's reviewed-by still stands so I will apply it. [...] Thank you! Krzysztof
[PATCH v6 5/5] PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c
Removed unused ".bus_shift" initialisers from pci-xgene.c as
xgene_pcie_map_bus() did not use these.
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
drivers/pci/controller/pci-xgene.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/pci/controller/pci-xgene.c
b/drivers/pci/controller/pci-xgene.c
index 8e0db84f089d..85e7c98265e8 100644
--- a/drivers/pci/controller/pci-xgene.c
+++ b/drivers/pci/controller/pci-xgene.c
@@ -257,7 +257,6 @@ static int xgene_v1_pcie_ecam_init(struct pci_config_window
*cfg)
}
const struct pci_ecam_ops xgene_v1_pcie_ecam_ops = {
- .bus_shift = 16,
.init = xgene_v1_pcie_ecam_init,
.pci_ops= {
.map_bus= xgene_pcie_map_bus,
@@ -272,7 +271,6 @@ static int xgene_v2_pcie_ecam_init(struct pci_config_window
*cfg)
}
const struct pci_ecam_ops xgene_v2_pcie_ecam_ops = {
- .bus_shift = 16,
.init = xgene_v2_pcie_ecam_init,
.pci_ops= {
.map_bus= xgene_pcie_map_bus,
--
2.29.2
[PATCH v6 4/5] PCI: vmd: Update type of the __iomem pointers
Use "void __iomem" instead "char __iomem" pointer type when working with
the accessor functions (with names like readb() or writel(), etc.) to
better match a given accessor function signature where commonly the
address pointing to an I/O memory region would be a "void __iomem"
pointer.
Related:
https://lwn.net/Articles/102232/
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
drivers/pci/controller/vmd.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 1361a79bd1e7..59fa9a94860f 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -95,7 +95,7 @@ struct vmd_dev {
struct pci_dev *dev;
spinlock_t cfg_lock;
- char __iomem*cfgbar;
+ void __iomem*cfgbar;
int msix_count;
struct vmd_irq_list *irqs;
@@ -326,7 +326,7 @@ static void vmd_remove_irq_domain(struct vmd_dev *vmd)
}
}
-static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
+static void __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
unsigned int devfn, int reg, int len)
{
unsigned int busnr_ecam = bus->number - vmd->busn_start;
@@ -346,7 +346,7 @@ static int vmd_pci_read(struct pci_bus *bus, unsigned int
devfn, int reg,
int len, u32 *value)
{
struct vmd_dev *vmd = vmd_from_bus(bus);
- char __iomem *addr = vmd_cfg_addr(vmd, bus, devfn, reg, len);
+ void __iomem *addr = vmd_cfg_addr(vmd, bus, devfn, reg, len);
unsigned long flags;
int ret = 0;
@@ -381,7 +381,7 @@ static int vmd_pci_write(struct pci_bus *bus, unsigned int
devfn, int reg,
int len, u32 value)
{
struct vmd_dev *vmd = vmd_from_bus(bus);
- char __iomem *addr = vmd_cfg_addr(vmd, bus, devfn, reg, len);
+ void __iomem *addr = vmd_cfg_addr(vmd, bus, devfn, reg, len);
unsigned long flags;
int ret = 0;
--
2.29.2
[PATCH v6 3/5] PCI: iproc: Convert to use the new ECAM constants
Change interface of the function iproc_pcie_map_ep_cfg_reg() so that use
of PCI_SLOT() and PCI_FUNC() macros and most of the local ECAM-specific
constants can be dropped, and the new PCIE_ECAM_OFFSET() macro can be
used instead. Use the ALIGN_DOWN() macro to ensure that PCI Express
ECAM offset is always 32 bit aligned.
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
drivers/pci/controller/pcie-iproc.c | 31 -
1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/drivers/pci/controller/pcie-iproc.c
b/drivers/pci/controller/pcie-iproc.c
index 905e93808243..503662380ff8 100644
--- a/drivers/pci/controller/pcie-iproc.c
+++ b/drivers/pci/controller/pcie-iproc.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#include
#include
#include
@@ -39,16 +40,8 @@
#define CFG_IND_ADDR_MASK 0x1ffc
-#define CFG_ADDR_BUS_NUM_SHIFT 20
-#define CFG_ADDR_BUS_NUM_MASK 0x0ff0
-#define CFG_ADDR_DEV_NUM_SHIFT 15
-#define CFG_ADDR_DEV_NUM_MASK 0x000f8000
-#define CFG_ADDR_FUNC_NUM_SHIFT12
-#define CFG_ADDR_FUNC_NUM_MASK 0x7000
-#define CFG_ADDR_REG_NUM_SHIFT 2
#define CFG_ADDR_REG_NUM_MASK 0x0ffc
-#define CFG_ADDR_CFG_TYPE_SHIFT0
-#define CFG_ADDR_CFG_TYPE_MASK 0x0003
+#define CFG_ADDR_CFG_TYPE_11
#define SYS_RC_INTX_MASK 0xf
@@ -459,19 +452,15 @@ static inline void iproc_pcie_apb_err_disable(struct
pci_bus *bus,
static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,
unsigned int busno,
- unsigned int slot,
- unsigned int fn,
+ unsigned int devfn,
int where)
{
u16 offset;
u32 val;
/* EP device access */
- val = (busno << CFG_ADDR_BUS_NUM_SHIFT) |
- (slot << CFG_ADDR_DEV_NUM_SHIFT) |
- (fn << CFG_ADDR_FUNC_NUM_SHIFT) |
- (where & CFG_ADDR_REG_NUM_MASK) |
- (1 & CFG_ADDR_CFG_TYPE_MASK);
+ val = ALIGN_DOWN(PCIE_ECAM_OFFSET(busno, devfn, where), 4) |
+ CFG_ADDR_CFG_TYPE_1;
iproc_pcie_write_reg(pcie, IPROC_PCIE_CFG_ADDR, val);
offset = iproc_pcie_reg_offset(pcie, IPROC_PCIE_CFG_DATA);
@@ -574,8 +563,6 @@ static int iproc_pcie_config_read(struct pci_bus *bus,
unsigned int devfn,
int where, int size, u32 *val)
{
struct iproc_pcie *pcie = iproc_data(bus);
- unsigned int slot = PCI_SLOT(devfn);
- unsigned int fn = PCI_FUNC(devfn);
unsigned int busno = bus->number;
void __iomem *cfg_data_p;
unsigned int data;
@@ -590,7 +577,7 @@ static int iproc_pcie_config_read(struct pci_bus *bus,
unsigned int devfn,
return ret;
}
- cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);
+ cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, devfn, where);
if (!cfg_data_p)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -631,13 +618,11 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct
iproc_pcie *pcie,
int busno, unsigned int devfn,
int where)
{
- unsigned slot = PCI_SLOT(devfn);
- unsigned fn = PCI_FUNC(devfn);
u16 offset;
/* root complex access */
if (busno == 0) {
- if (slot > 0 || fn > 0)
+ if (PCIE_ECAM_DEVFN(devfn) > 0)
return NULL;
iproc_pcie_write_reg(pcie, IPROC_PCIE_CFG_IND_ADDR,
@@ -649,7 +634,7 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct
iproc_pcie *pcie,
return (pcie->base + offset);
}
- return iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);
+ return iproc_pcie_map_ep_cfg_reg(pcie, busno, devfn, where);
}
static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
--
2.29.2
[PATCH v6 2/5] PCI: thunder-pem: Add constant for custom ".bus_shit" initialiser
Add a custom constant for the ".bus_shit" initialiser to capture
a non-standard platform-specific ECAM bus shift value.
Standard values otherwise defined in the PCI Express Specification
are available in the include/linux/pci-ecam.h.
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
drivers/pci/controller/pci-thunder-pem.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pci-thunder-pem.c
b/drivers/pci/controller/pci-thunder-pem.c
index 3f847969143e..1a3f70ac61fc 100644
--- a/drivers/pci/controller/pci-thunder-pem.c
+++ b/drivers/pci/controller/pci-thunder-pem.c
@@ -19,6 +19,15 @@
#define PEM_CFG_WR 0x28
#define PEM_CFG_RD 0x30
+/*
+ * Enhanced Configuration Access Mechanism (ECAM)
+ *
+ * N.B. This is a non-standard platform-specific ECAM bus shift value. For
+ * standard values defined in the PCI Express Base Specification see
+ * include/linux/pci-ecam.h.
+ */
+#define THUNDER_PCIE_ECAM_BUS_SHIFT24
+
struct thunder_pem_pci {
u32 ea_entry[3];
void __iomem*pem_reg_base;
@@ -404,7 +413,7 @@ static int thunder_pem_acpi_init(struct pci_config_window
*cfg)
}
const struct pci_ecam_ops thunder_pem_ecam_ops = {
- .bus_shift = 24,
+ .bus_shift = THUNDER_PCIE_ECAM_BUS_SHIFT,
.init = thunder_pem_acpi_init,
.pci_ops= {
.map_bus= pci_ecam_map_bus,
@@ -441,7 +450,7 @@ static int thunder_pem_platform_init(struct
pci_config_window *cfg)
}
static const struct pci_ecam_ops pci_thunder_pem_ops = {
- .bus_shift = 24,
+ .bus_shift = THUNDER_PCIE_ECAM_BUS_SHIFT,
.init = thunder_pem_platform_init,
.pci_ops= {
.map_bus= pci_ecam_map_bus,
--
2.29.2
[PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers
Unify ECAM-related constants into a single set of standard constants
defining memory address shift values for the byte-level address that can
be used when accessing the PCI Express Configuration Space, and then
move native PCI Express controller drivers to use newly introduced
definitions retiring any driver-specific ones.
The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
PCI Express specification (see PCI Express Base Specification, Revision
5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
implement it the same way.
Most of the native PCI Express controller drivers define their ECAM-related
constants, many of these could be shared, or use open-coded values when
setting the ".bus_shift" field of the "struct pci_ecam_ops".
All of the newly added constants should remove ambiguity and reduce the
number of open-coded values, and also correlate more strongly with the
descriptions in the aforementioned specification (see Table 7-1
"Enhanced Configuration Address Mapping", p. 677).
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
Changed in v6:
Converted single patch into a series.
Dropped changes related to PPC 4xx platform.
Refactored pci_ecam_map_bus() so that bus, device function and offset
are correctly masked, limiting offset to 4K as per the PCI Express
Specification. After the refactor this function will now use sensible
defaults allowing for removal of the ".bus_shit" initialiser from all
the users of the "ecam_ops" structure who do not use a non-standard
ECAM bus shit values.
Changed in v5:
Removed unused constant "PCIE_ECAM_DEV_SHIFT".
Refactored ppc4xx_pciex_get_config_base() so that the "offset"
parameter can be passed to so that the PCIE_ECAM_OFFSET() macro
can be used.
Used the ALIGN_DOWN() macro where 32 bit alignment is required
instead using the 0xffc mask.
Added CFG_ADDR_CFG_TYPE_1 macro to pci/controller/pcie-iproc.c to
denote that this is a configuration type 1 address and access type.
Refactored boundary check in pci/controller/vmd.c as used by the
vmd_cfg_addr() function following addition of the PCIE_ECAM_OFFSET()
macro.
Changed the "bus->number" to simply pass the "bus" argument in the
PCIE_ECAM_OFFSET() macro.
Changed in v4:
Removed constants related to "CAM".
Added more platforms and devices that can use new ECAM macros and
constants.
Removed unused ".bus_shift" initialisers from pci-xgene.c as
xgene_pcie_map_bus() did not use these.
Changes in v3:
Updated commit message wording.
Updated regarding custom ECAM bus shift values and concerning PCI base
configuration space access for Type 1 access.
Refactored rockchip_pcie_rd_other_conf() and rockchip_pcie_wr_other_conf()
and removed the "busdev" variable.
Removed surplus "relbus" variable from nwl_pcie_map_bus() and
xilinx_pcie_map_bus().
Renamed the PCIE_ECAM_ADDR() macro to PCIE_ECAM_OFFSET().
Changes in v2:
Use PCIE_ECAM_ADDR macro when computing ECAM address offset, but drop
PCI_SLOT and PCI_FUNC macros from the PCIE_ECAM_ADDR macro in favour
of using a single value for the device/function.
Krzysztof Wilczyński (5):
PCI: Unify ECAM constants in native PCI Express drivers
PCI: thunder-pem: Add constant for custom ".bus_shit" initialiser
PCI: iproc: Convert to use the new ECAM constants
PCI: vmd: Update type of the __iomem pointers
PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c
drivers/pci/controller/dwc/pcie-al.c| 12 ++--
drivers/pci/controller/dwc/pcie-hisi.c | 2 --
drivers/pci/controller/pci-aardvark.c | 13 ++---
drivers/pci/controller/pci-host-generic.c | 1 -
drivers/pci/controller/pci-thunder-ecam.c | 1 -
drivers/pci/controller/pci-thunder-pem.c| 13 +++--
drivers/pci/controller/pci-xgene.c | 2 --
drivers/pci/controller/pcie-brcmstb.c | 16 ++-
drivers/pci/controller/pcie-iproc.c | 31 ++---
drivers/pci/controller/pcie-rockchip-host.c | 27 +-
drivers/pci/controller/pcie-rockchip.h | 8 +-
drivers/pci/controller/pcie-tango.c | 1 -
drivers/pci/controller/pcie-xilinx-nwl.c| 9 ++
drivers/pci/controller/pcie-xilinx.c| 11 ++--
drivers/pci/controller/vmd.c| 19 ++---
drivers/pci/ecam.c | 23 ++-
include/linux/pci-ecam.h| 27 ++
17 files changed, 96 insertions(+), 120 deletions(-)
--
2.29.2
[PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
Add ECAM-related constants to provide a set of standard constants
defining memory address shift values to the byte-level address that can
be used to access the PCI Express Configuration Space, and then move
native PCI Express controller drivers to use the newly introduced
definitions retiring driver-specific ones.
Refactor pci_ecam_map_bus() function to use newly added constants so
that limits to the bus, device function and offset (now limited to 4K as
per the specification) are in place to prevent the defective or
malicious caller from supplying incorrect configuration offset and thus
targeting the wrong device when accessing extended configuration space.
This refactor also allows for the ".bus_shit" initialisers to be dropped
when the user is not using a custom value as a default value will be
used as per the PCI Express Specification.
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
drivers/pci/controller/dwc/pcie-al.c| 12 ++---
drivers/pci/controller/dwc/pcie-hisi.c | 2 --
drivers/pci/controller/pci-aardvark.c | 13 +++---
drivers/pci/controller/pci-host-generic.c | 1 -
drivers/pci/controller/pci-thunder-ecam.c | 1 -
drivers/pci/controller/pcie-brcmstb.c | 16 ++--
drivers/pci/controller/pcie-rockchip-host.c | 27 ++---
drivers/pci/controller/pcie-rockchip.h | 8 +-
drivers/pci/controller/pcie-tango.c | 1 -
drivers/pci/controller/pcie-xilinx-nwl.c| 9 ++-
drivers/pci/controller/pcie-xilinx.c| 11 ++---
drivers/pci/controller/vmd.c| 11 -
drivers/pci/ecam.c | 23 --
include/linux/pci-ecam.h| 27 +
14 files changed, 73 insertions(+), 89 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-al.c
b/drivers/pci/controller/dwc/pcie-al.c
index f973fbca90cf..af9e51ab1af8 100644
--- a/drivers/pci/controller/dwc/pcie-al.c
+++ b/drivers/pci/controller/dwc/pcie-al.c
@@ -76,7 +76,6 @@ static int al_pcie_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops al_pcie_ops = {
- .bus_shift= 20,
.init = al_pcie_init,
.pci_ops = {
.map_bus= al_pcie_map_bus,
@@ -138,8 +137,6 @@ struct al_pcie {
struct al_pcie_target_bus_cfg target_bus_cfg;
};
-#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << 12)
-
#define to_al_pcie(x) dev_get_drvdata((x)->dev)
static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
@@ -226,11 +223,6 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct
pci_bus *bus,
struct al_pcie_target_bus_cfg *target_bus_cfg = >target_bus_cfg;
unsigned int busnr_ecam = busnr & target_bus_cfg->ecam_mask;
unsigned int busnr_reg = busnr & target_bus_cfg->reg_mask;
- void __iomem *pci_base_addr;
-
- pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
-(busnr_ecam << 20) +
-PCIE_ECAM_DEVFN(devfn));
if (busnr_reg != target_bus_cfg->reg_val) {
dev_dbg(pcie->pci->dev, "Changing target bus busnum val from
0x%x to 0x%x\n",
@@ -241,7 +233,7 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct
pci_bus *bus,
target_bus_cfg->reg_mask);
}
- return pci_base_addr + where;
+ return pp->va_cfg0_base + PCIE_ECAM_OFFSET(busnr_ecam, devfn, where);
}
static struct pci_ops al_child_pci_ops = {
@@ -264,7 +256,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
target_bus_cfg = >target_bus_cfg;
- ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
+ ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
if (ecam_bus_mask > 255) {
dev_warn(pcie->dev, "ECAM window size is larger than 256MB.
Cutting off at 256\n");
ecam_bus_mask = 255;
diff --git a/drivers/pci/controller/dwc/pcie-hisi.c
b/drivers/pci/controller/dwc/pcie-hisi.c
index 5ca86796d43a..8fc5960faf28 100644
--- a/drivers/pci/controller/dwc/pcie-hisi.c
+++ b/drivers/pci/controller/dwc/pcie-hisi.c
@@ -100,7 +100,6 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops hisi_pcie_ops = {
- .bus_shift= 20,
.init = hisi_pcie_init,
.pci_ops = {
.map_bus= hisi_pcie_map_bus,
@@ -135,7 +134,6 @@ static int hisi_pcie_platform_init(struct pci_config_window
*cfg)
}
static const struct pci_ecam_ops hisi_pcie_platform_ops = {
- .bus_shift= 20,
.init = hisi_pcie_platform_init,
.pci_ops = {
.map_bus= hisi_pcie_map_bus,
diff --git a/drivers/pci/controller/pci-aardvark.c
b
[PATCH v5] PCI: Unify ECAM constants in native PCI Express drivers
Unify ECAM-related constants into a single set of standard constants
defining memory address shift values for the byte-level address that can
be used when accessing the PCI Express Configuration Space, and then
move native PCI Express controller drivers to use newly introduced
definitions retiring any driver-specific ones.
The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
PCI Express specification (see PCI Express Base Specification, Revision
5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
implement it the same way. Most of the native PCI Express controller
drivers define their ECAM-related constants, many of these could be
shared, or use open-coded values when setting the .bus_shift field of
the struct pci_ecam_ops.
All of the newly added constants should remove ambiguity and reduce the
number of open-coded values, and also correlate more strongly with the
descriptions in the aforementioned specification (see Table 7-1
"Enhanced Configuration Address Mapping", p. 677).
There is no change to functionality.
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
Changed in v5:
Removed unused constant "PCIE_ECAM_DEV_SHIFT".
Refactored ppc4xx_pciex_get_config_base() so that the "offset"
parameter can be passed to so that the PCIE_ECAM_OFFSET() macro
can be used.
Used the ALIGN_DOWN() macro where 32 bit alignment is required
instead using the 0xffc mask.
Added CFG_ADDR_CFG_TYPE_1 macro to pci/controller/pcie-iproc.c to
denote that this is a configuration type 1 address and access type.
Refactored boundary check in pci/controller/vmd.c as used by the
vmd_cfg_addr() function following addition of the PCIE_ECAM_OFFSET()
macro.
Changed the "bus->number" to simply pass the "bus" argument in the
PCIE_ECAM_OFFSET() macro.
Changed in v4:
Removed constants related to "CAM".
Added more platforms and devices that can use new ECAM macros and
constants.
Removed unused ".bus_shift" initialisers from pci-xgene.c as
xgene_pcie_map_bus() did not use these.
Changes in v3:
Updated commit message wording.
Updated regarding custom ECAM bus shift values and concerning PCI base
configuration space access for Type 1 access.
Refactored rockchip_pcie_rd_other_conf() and rockchip_pcie_wr_other_conf()
and removed the "busdev" variable.
Removed surplus "relbus" variable from nwl_pcie_map_bus() and
xilinx_pcie_map_bus().
Renamed the PCIE_ECAM_ADDR() macro to PCIE_ECAM_OFFSET().
Changes in v2:
Use PCIE_ECAM_ADDR macro when computing ECAM address offset, but drop
PCI_SLOT and PCI_FUNC macros from the PCIE_ECAM_ADDR macro in favour
of using a single value for the device/function.
arch/powerpc/platforms/4xx/pci.c| 30 ++--
drivers/pci/controller/dwc/pcie-al.c| 13 ++---
drivers/pci/controller/dwc/pcie-hisi.c | 4 +--
drivers/pci/controller/pci-aardvark.c | 13 ++---
drivers/pci/controller/pci-host-generic.c | 2 +-
drivers/pci/controller/pci-thunder-ecam.c | 2 +-
drivers/pci/controller/pci-thunder-pem.c| 13 +++--
drivers/pci/controller/pci-xgene.c | 2 --
drivers/pci/controller/pcie-brcmstb.c | 16 ++-
drivers/pci/controller/pcie-iproc.c | 31 ++---
drivers/pci/controller/pcie-rockchip-host.c | 27 +-
drivers/pci/controller/pcie-rockchip.h | 8 +-
drivers/pci/controller/pcie-tango.c | 2 +-
drivers/pci/controller/pcie-xilinx-nwl.c| 9 ++
drivers/pci/controller/pcie-xilinx.c| 11 ++--
drivers/pci/controller/vmd.c| 19 ++---
drivers/pci/ecam.c | 4 +--
include/linux/pci-ecam.h| 23 +++
18 files changed, 99 insertions(+), 130 deletions(-)
diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
index c13d64c3b019..4f3c529ef3b9 100644
--- a/arch/powerpc/platforms/4xx/pci.c
+++ b/arch/powerpc/platforms/4xx/pci.c
@@ -20,6 +20,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1583,9 +1584,10 @@ static int ppc4xx_pciex_validate_bdf(struct
ppc4xx_pciex_port *port,
static void __iomem *ppc4xx_pciex_get_config_base(struct ppc4xx_pciex_port
*port,
struct pci_bus *bus,
- unsigned int devfn)
+ unsigned int devfn,
+ int offset)
{
- int relbus;
+ unsigned int busnr_ecam = bus->number - (port->hose->first_busno + 1);
/* Remove the casts when we finally remove the stupid volatile
* in struct pci_controller
@@ -1593,9 +1595,7 @@ static void __iomem *ppc4xx_pciex_get_config_base(struct
ppc4xx_pciex_
Re: [PATCH v4] PCI: Unify ECAM constants in native PCI Express drivers
On 20-10-04 19:53:06, Florian Fainelli wrote: Hi Florian, Sorry for taking a long time to get back to you. [...] > This appears to be correct, so: > > Acked-by: Florian Fainelli Thank you! > however, I would have defined a couple of additional helper macros and do: > > idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEV(devfn) | > PCIE_ECAM_FUN(devfn); > > for clarity. > [...] > For instance, adding these two: > > #define PCIE_ECAM_DEV(x) (((x) & 0x1f) << PCIE_ECAM_DEV_SHIFT) > #define PCIE_ECAM_FUN(x) (((x) & 0x7) << PCIE_ECAM_FUN_SHIFT) > > may be clearer for use in drivers like pcie-brcmstb.c that used to treat the > device function in terms of device and function (though it was called slot > there). Regarding the suggestion above - it has been like that initially, albeit Bjorn suggested that there is no need to reply on the macros that use PCI_SLOT() and PCI_FUNC() macros, see: https://lore.kernel.org/linux-pci/20200922232715.GA2238688@bjorn-Precision-5520/ I would be happy to put the macros back if there is a value in having the extra macros added - perhaps for clarify, as you suggest. Krzysztof
[PATCH v4] PCI: Unify ECAM constants in native PCI Express drivers
Unify ECAM-related constants into a single set of standard constants
defining memory address shift values for the byte-level address that can
be used when accessing the PCI Express Configuration Space, and then
move native PCI Express controller drivers to use newly introduced
definitions retiring any driver-specific ones.
The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
PCI Express specification (see PCI Express Base Specification, Revision
5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
implement it the same way. Most of the native PCI Express controller
drivers define their ECAM-related constants, many of these could be
shared, or use open-coded values when setting the .bus_shift field of
the struct pci_ecam_ops.
All of the newly added constants should remove ambiguity and reduce the
number of open-coded values, and also correlate more strongly with the
descriptions in the aforementioned specification (see Table 7-1
"Enhanced Configuration Address Mapping", p. 677).
There is no change to functionality.
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
Changed in v4:
Removed constants related to "CAM".
Added more platforms and devices that can use new ECAM macros and
constants.
Removed unused ".bus_shift" initialisers from pci-xgene.c as
xgene_pcie_map_bus() did not use these.
Changes in v3:
Updated commit message wording.
Updated regarding custom ECAM bus shift values and concerning PCI base
configuration space access for Type 1 access.
Refactored rockchip_pcie_rd_other_conf() and rockchip_pcie_wr_other_conf()
and removed the "busdev" variable.
Removed surplus "relbus" variable from nwl_pcie_map_bus() and
xilinx_pcie_map_bus().
Renamed the PCIE_ECAM_ADDR() macro to PCIE_ECAM_OFFSET().
Changes in v2:
Use PCIE_ECAM_ADDR macro when computing ECAM address offset, but drop
PCI_SLOT and PCI_FUNC macros from the PCIE_ECAM_ADDR macro in favour
of using a single value for the device/function.
arch/powerpc/platforms/4xx/pci.c| 7 +++--
drivers/pci/controller/dwc/pcie-al.c| 8 +++---
drivers/pci/controller/dwc/pcie-hisi.c | 4 +--
drivers/pci/controller/pci-aardvark.c | 13 +++--
drivers/pci/controller/pci-host-generic.c | 2 +-
drivers/pci/controller/pci-thunder-ecam.c | 2 +-
drivers/pci/controller/pci-thunder-pem.c| 13 +++--
drivers/pci/controller/pci-xgene.c | 2 --
drivers/pci/controller/pcie-brcmstb.c | 16 ++--
drivers/pci/controller/pcie-iproc.c | 29 ++---
drivers/pci/controller/pcie-rockchip-host.c | 27 +--
drivers/pci/controller/pcie-rockchip.h | 8 +-
drivers/pci/controller/pcie-tango.c | 2 +-
drivers/pci/controller/pcie-xilinx-nwl.c| 9 ++-
drivers/pci/controller/pcie-xilinx.c| 11 ++--
drivers/pci/controller/vmd.c| 5 ++--
drivers/pci/ecam.c | 4 +--
include/linux/pci-ecam.h| 24 +
18 files changed, 82 insertions(+), 104 deletions(-)
diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
index c13d64c3b019..cee40e0b061c 100644
--- a/arch/powerpc/platforms/4xx/pci.c
+++ b/arch/powerpc/platforms/4xx/pci.c
@@ -20,6 +20,7 @@
#include
#include
+#include
#include
#include
#include
@@ -1585,17 +1586,15 @@ static void __iomem
*ppc4xx_pciex_get_config_base(struct ppc4xx_pciex_port *port
struct pci_bus *bus,
unsigned int devfn)
{
- int relbus;
-
/* Remove the casts when we finally remove the stupid volatile
* in struct pci_controller
*/
if (bus->number == port->hose->first_busno)
return (void __iomem *)port->hose->cfg_addr;
- relbus = bus->number - (port->hose->first_busno + 1);
return (void __iomem *)port->hose->cfg_data +
- ((relbus << 20) | (devfn << 12));
+ PCIE_ECAM_BUS(bus->number - (port->hose->first_busno + 1)) |
+ PCIE_ECAM_DEVFN(devfn);
}
static int ppc4xx_pciex_read_config(struct pci_bus *bus, unsigned int devfn,
diff --git a/drivers/pci/controller/dwc/pcie-al.c
b/drivers/pci/controller/dwc/pcie-al.c
index d57d4ee15848..7c2aa049113c 100644
--- a/drivers/pci/controller/dwc/pcie-al.c
+++ b/drivers/pci/controller/dwc/pcie-al.c
@@ -76,7 +76,7 @@ static int al_pcie_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops al_pcie_ops = {
- .bus_shift= 20,
+ .bus_shift= PCIE_ECAM_BUS_SHIFT,
.init = al_pcie_init,
.pci_ops = {
.map_bus= al_pcie_map_bus,
@@ -138,8 +138,6 @@ struct al_pcie {
struct al_pcie_target_bus
[PATCH] PCI: Convert enum pci_bus_flags to bit fields in struct pci_bus
All the flags defined in the enum pci_bus_flags are used to determine
whether a particular feature of a PCI bus is available or not - features
are also often disabled via architecture or device-specific quirk.
These flags are tightly coupled with a PCI buses and PCI bridges and
primarily used in simple binary on/off manner to check whether something
is enabled or disabled, and have almost no other users (with an
exception of the x86 architecture-specific quirk) outside of the PCI
device drivers space.
Therefore, convert enum pci_bus_flags into a set of bit fields in the
struct pci_bus, and then drop said enum and the typedef pci_bus_flags_t.
This will keep PCI device-specific features as part of the struct
pci_dev and make the code that used to use flags simpler.
Related:
https://patchwork.kernel.org/patch/11772809
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
arch/x86/pci/fixup.c| 6 +++---
drivers/pci/msi.c | 8
drivers/pci/pci-sysfs.c | 14 ++
drivers/pci/pci.c | 2 +-
drivers/pci/pcie/aer.c | 5 ++---
drivers/pci/probe.c | 13 +
drivers/pci/quirks.c| 16
include/linux/pci.h | 20 ++--
8 files changed, 43 insertions(+), 41 deletions(-)
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b8c9a5b87f37..50ff8aa542b8 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -641,14 +641,14 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x8c10,
quirk_apple_mbp_poweroff);
* ID, the AER driver should traverse the child device tree, reading
* AER registers to find the faulting device.
*/
-static void quirk_no_aersid(struct pci_dev *pdev)
+static void quirk_no_aer_sid(struct pci_dev *pdev)
{
/* VMD Domain */
if (is_vmd(pdev->bus) && pci_is_root_bus(pdev->bus))
- pdev->bus->bus_flags |= PCI_BUS_FLAGS_NO_AERSID;
+ pdev->bus->no_aer_sid = 1;
}
DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
- PCI_CLASS_BRIDGE_PCI, 8, quirk_no_aersid);
+ PCI_CLASS_BRIDGE_PCI, 8, quirk_no_aer_sid);
static void quirk_intel_th_dnv(struct pci_dev *dev)
{
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 30ae4ffda5c1..01e4bdbc830e 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -875,13 +875,13 @@ static int pci_msi_supported(struct pci_dev *dev, int
nvec)
/*
* Any bridge which does NOT route MSI transactions from its
-* secondary bus to its primary bus must set NO_MSI flag on
-* the secondary pci_bus.
+* secondary bus to its primary bus must enable "no_msi" on
+* the secondary bus (pci_bus).
* We expect only arch-specific PCI host bus controller driver
-* or quirks for specific PCI bridges to be setting NO_MSI.
+* or quirks for specific PCI bridges to enable "no_msi".
*/
for (bus = dev->bus; bus; bus = bus->parent)
- if (bus->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+ if (bus->no_msi)
return 0;
return 1;
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 6d78df981d41..eca214e45418 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -357,9 +357,7 @@ static ssize_t msi_bus_show(struct device *dev, struct
device_attribute *attr,
struct pci_dev *pdev = to_pci_dev(dev);
struct pci_bus *subordinate = pdev->subordinate;
- return sprintf(buf, "%u\n", subordinate ?
- !(subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI)
- : !pdev->no_msi);
+ return sprintf(buf, "%u\n", subordinate ? !subordinate->no_msi :
!pdev->no_msi);
}
static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
@@ -376,9 +374,9 @@ static ssize_t msi_bus_store(struct device *dev, struct
device_attribute *attr,
return -EPERM;
/*
-* "no_msi" and "bus_flags" only affect what happens when a driver
-* requests MSI or MSI-X. They don't affect any drivers that have
-* already requested MSI or MSI-X.
+* "no_msi" enabled for device and bus only affect what happens
+* when a driver requests MSI or MSI-X. They don't affect any
+* drivers that have already requested MSI or MSI-X.
*/
if (!subordinate) {
pdev->no_msi = !val;
@@ -388,9 +386,9 @@ static ssize_t msi_bus_store(struct device *dev, struct
device_attribute *attr,
}
if (val)
- subordinate->bus_flags &= ~PCI_BUS_FLAGS_NO_MSI;
+ subordinate->no_msi = 0;
else
- subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+ subordinate->no
