On Wed, 19 Jun 2024, Krishna Chaitanya Chundru wrote:
>
>
> On 6/14/2024 6:02 PM, Ilpo Järvinen wrote:
> > On Sun, 9 Jun 2024, Krishna chaitanya chundru wrote:
> >
> > > Bring the switch case in pcie_link_speed_mbps() to new function to
> > > the header file so that it can be used in other places like
> > > in controller driver.
> > >
> > > Signed-off-by: Krishna chaitanya chundru <quic_krichai@xxxxxxxxxxx>
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> > > Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > > ---
> > > drivers/pci/pci.c | 19 +------------------
> > > drivers/pci/pci.h | 22 ++++++++++++++++++++++
> > > 2 files changed, 23 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index d2c388761ba9..6e50fa89b913 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -6027,24 +6027,7 @@ int pcie_link_speed_mbps(struct pci_dev *pdev)
> > > if (err)
> > > return err;
> > > - switch (to_pcie_link_speed(lnksta)) {
> > > - case PCIE_SPEED_2_5GT:
> > > - return 2500;
> > > - case PCIE_SPEED_5_0GT:
> > > - return 5000;
> > > - case PCIE_SPEED_8_0GT:
> > > - return 8000;
> > > - case PCIE_SPEED_16_0GT:
> > > - return 16000;
> > > - case PCIE_SPEED_32_0GT:
> > > - return 32000;
> > > - case PCIE_SPEED_64_0GT:
> > > - return 64000;
> > > - default:
> > > - break;
> > > - }
> > > -
> > > - return -EINVAL;
> > > + return pcie_link_speed_to_mbps(to_pcie_link_speed(lnksta));
> >
> > pcie_link_speed_mbps() calls pcie_link_speed_to_mbps(), seems quite
> > confusing to me. Perhaps renaming one to pcie_dev_speed_mbps() would help
> > against the almost identical naming.
> >
> > In general, I don't like moving that code into a header file, did you
> > check how large footprint the new function is (when it's not inlined)?
> >
> if we remove this patch we see difference of 8, I think it should be fine.
> with change
> aarch64-linux-gnu-size ../drivers/pci/pci.o
> text data bss dec hex filename
> 41440 1334 64 42838 a756 ../kobj/drivers/pci/pci.o
> without the change
> text data bss dec hex filename
> 41432 1334 64 42830 a74e ../kobj/drivers/pci/pci.o
Thanks for checking it out, seems the inline here was a non-problem.
--
i.
> > Unrelated to this patch, it would be nice if LNKSTA register read would
> > not be needed at all here but since cur_bus_speed is what it is currently,
> > it's just wishful thinking.
> >
> > > }
> > > EXPORT_SYMBOL(pcie_link_speed_mbps);
> > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > > index 1b021579f26a..391a5cd388bd 100644
> > > --- a/drivers/pci/pci.h
> > > +++ b/drivers/pci/pci.h
> > > @@ -333,6 +333,28 @@ void pci_bus_put(struct pci_bus *bus);
> > > (speed) == PCIE_SPEED_2_5GT ? 2500*8/10 : \
> > > 0)
> > > +static inline int pcie_link_speed_to_mbps(enum pci_bus_speed speed)
> > > +{
> > > + switch (speed) {
> > > + case PCIE_SPEED_2_5GT:
> > > + return 2500;
> > > + case PCIE_SPEED_5_0GT:
> > > + return 5000;
> > > + case PCIE_SPEED_8_0GT:
> > > + return 8000;
> > > + case PCIE_SPEED_16_0GT:
> > > + return 16000;
> > > + case PCIE_SPEED_32_0GT:
> > > + return 32000;
> > > + case PCIE_SPEED_64_0GT:
> > > + return 64000;
> > > + default:
> > > + break;
> > > + }
> > > +
> > > + return -EINVAL;
> > > +}
> >
> >
> >
>
