| Message ID | 20241017095545.1424-1-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 19f73e938df2b8edfa2e93e0280bd26fd4df5b92 |
| Headers | show |
| Series | [v2,1/1] PCI: Improve printout in pdev_sort_resources() | expand |
On Thu, 17 Oct 2024 12:55:45 +0300 Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > Use pci_resource_name() helper in pdev_sort_resources() to print > resources in user-friendly format. Also replace the vague "bogus > alignment" with a more precise explanation of the problem. > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > --- > > v2: > - Place colon after %s %pR to be consistent with other printouts > - Replace vague "bogus alignment" with the exact cause > > drivers/pci/setup-bus.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 23082bc0ca37..0fd286f79674 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -134,6 +134,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) > int i; > > pci_dev_for_each_resource(dev, r, i) { > + const char *r_name = pci_resource_name(dev, i); > struct pci_dev_resource *dev_res, *tmp; > resource_size_t r_align; > struct list_head *n; > @@ -146,8 +147,8 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) > > r_align = pci_resource_alignment(dev, r); > if (!r_align) { > - pci_warn(dev, "BAR %d: %pR has bogus alignment\n", > - i, r); > + pci_warn(dev, "%s %pR: alignment must not be zero\n", > + r_name, r); Why bother with local variable if only used here? Absolutely fine if you have more code coming that uses it again though! Otherwise seems sensible change. > continue; > } >
On Thu, 17 Oct 2024, Jonathan Cameron wrote: > On Thu, 17 Oct 2024 12:55:45 +0300 > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > Use pci_resource_name() helper in pdev_sort_resources() to print > > resources in user-friendly format. Also replace the vague "bogus > > alignment" with a more precise explanation of the problem. > > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > --- > > > > v2: > > - Place colon after %s %pR to be consistent with other printouts > > - Replace vague "bogus alignment" with the exact cause > > > > drivers/pci/setup-bus.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > index 23082bc0ca37..0fd286f79674 100644 > > --- a/drivers/pci/setup-bus.c > > +++ b/drivers/pci/setup-bus.c > > @@ -134,6 +134,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) > > int i; > > > > pci_dev_for_each_resource(dev, r, i) { > > + const char *r_name = pci_resource_name(dev, i); > > struct pci_dev_resource *dev_res, *tmp; > > resource_size_t r_align; > > struct list_head *n; > > @@ -146,8 +147,8 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) > > > > r_align = pci_resource_alignment(dev, r); > > if (!r_align) { > > - pci_warn(dev, "BAR %d: %pR has bogus alignment\n", > > - i, r); > > + pci_warn(dev, "%s %pR: alignment must not be zero\n", > > + r_name, r); > > Why bother with local variable if only used here? No other reason than it seems to always be a local variable in the other places too regardless the number of uses. > Absolutely fine if you have more code coming that uses it again though! > > Otherwise seems sensible change.
On Thu, 17 Oct 2024 14:35:55 +0300 (EEST) Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > On Thu, 17 Oct 2024, Jonathan Cameron wrote: > > > On Thu, 17 Oct 2024 12:55:45 +0300 > > Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote: > > > > > Use pci_resource_name() helper in pdev_sort_resources() to print > > > resources in user-friendly format. Also replace the vague "bogus > > > alignment" with a more precise explanation of the problem. > > > > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > > --- > > > > > > v2: > > > - Place colon after %s %pR to be consistent with other printouts > > > - Replace vague "bogus alignment" with the exact cause > > > > > > drivers/pci/setup-bus.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > > index 23082bc0ca37..0fd286f79674 100644 > > > --- a/drivers/pci/setup-bus.c > > > +++ b/drivers/pci/setup-bus.c > > > @@ -134,6 +134,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) > > > int i; > > > > > > pci_dev_for_each_resource(dev, r, i) { > > > + const char *r_name = pci_resource_name(dev, i); > > > struct pci_dev_resource *dev_res, *tmp; > > > resource_size_t r_align; > > > struct list_head *n; > > > @@ -146,8 +147,8 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) > > > > > > r_align = pci_resource_alignment(dev, r); > > > if (!r_align) { > > > - pci_warn(dev, "BAR %d: %pR has bogus alignment\n", > > > - i, r); > > > + pci_warn(dev, "%s %pR: alignment must not be zero\n", > > > + r_name, r); > > > > Why bother with local variable if only used here? > > No other reason than it seems to always be a local variable in the other > places too regardless the number of uses. Fair enough. local style is perfectly valid reasoning. Jonathan > > > Absolutely fine if you have more code coming that uses it again though! > > > > Otherwise seems sensible change. >
On Thu, 2024-10-17 at 12:55 +0300, Ilpo Järvinen wrote: > Use pci_resource_name() helper in pdev_sort_resources() to print > resources in user-friendly format. Also replace the vague "bogus > alignment" with a more precise explanation of the problem. > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Reviewed-by: Philipp Stanner <pstanner@redhat.com> > --- > > v2: > - Place colon after %s %pR to be consistent with other printouts > - Replace vague "bogus alignment" with the exact cause > > drivers/pci/setup-bus.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 23082bc0ca37..0fd286f79674 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -134,6 +134,7 @@ static void pdev_sort_resources(struct pci_dev > *dev, struct list_head *head) > int i; > > pci_dev_for_each_resource(dev, r, i) { > + const char *r_name = pci_resource_name(dev, i); > struct pci_dev_resource *dev_res, *tmp; > resource_size_t r_align; > struct list_head *n; > @@ -146,8 +147,8 @@ static void pdev_sort_resources(struct pci_dev > *dev, struct list_head *head) > > r_align = pci_resource_alignment(dev, r); > if (!r_align) { > - pci_warn(dev, "BAR %d: %pR has bogus > alignment\n", > - i, r); > + pci_warn(dev, "%s %pR: alignment must not be > zero\n", > + r_name, r); > continue; > } >
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index 23082bc0ca37..0fd286f79674 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -134,6 +134,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) int i; pci_dev_for_each_resource(dev, r, i) { + const char *r_name = pci_resource_name(dev, i); struct pci_dev_resource *dev_res, *tmp; resource_size_t r_align; struct list_head *n; @@ -146,8 +147,8 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) r_align = pci_resource_alignment(dev, r); if (!r_align) { - pci_warn(dev, "BAR %d: %pR has bogus alignment\n", - i, r); + pci_warn(dev, "%s %pR: alignment must not be zero\n", + r_name, r); continue; }
Use pci_resource_name() helper in pdev_sort_resources() to print resources in user-friendly format. Also replace the vague "bogus alignment" with a more precise explanation of the problem. Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> --- v2: - Place colon after %s %pR to be consistent with other printouts - Replace vague "bogus alignment" with the exact cause drivers/pci/setup-bus.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)