[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <e501971f-e9e1-d87a-ca44-b2be61f79f3e@linux.intel.com>
Date: Sun, 23 Jun 2024 18:26:03 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Krishna Chaitanya Chundru <quic_krichai@...cinc.com>
cc: Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Bjorn Helgaas <bhelgaas@...gle.com>, johan+linaro@...nel.org,
bmasney@...hat.com, djakov@...nel.org,
Krzysztof Kozlowski <krzk+dt@...nel.org>, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
linux-pci@...r.kernel.org, vireshk@...nel.org, quic_vbadigan@...cinc.com,
quic_skananth@...cinc.com, quic_nitegupt@...cinc.com,
quic_parass@...cinc.com, krzysztof.kozlowski@...aro.org
Subject: Re: [PATCH v14 3/4] PCI: Bring the PCIe speed to MBps logic to new
pcie_link_speed_to_mbps()
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@...cinc.com>
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > > Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>
> > > ---
> > > 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;
> > > +}
> >
> >
> >
>
Powered by blists - more mailing lists