Re: [PATCH v4 3/3] pci: intel: Add sysfs attributes to configure pcie link
[+cc Rafael, linux-pm, beginning of discussion at https://lore.kernel.org/r/d8574605f8e70f41ce1e88ccfb56b63c8f85e4df.1571638827.git.eswara.k...@linux.intel.com] On Tue, Oct 22, 2019 at 05:27:38PM +0800, Dilip Kota wrote: > On 10/22/2019 1:18 AM, Bjorn Helgaas wrote: > > On Mon, Oct 21, 2019 at 02:38:50PM +0100, Andrew Murray wrote: > > > On Mon, Oct 21, 2019 at 02:39:20PM +0800, Dilip Kota wrote: > > > > PCIe RC driver on Intel Gateway SoCs have a requirement > > > > of changing link width and speed on the fly. > > Please add more details about why this is needed. Since you're adding > > sysfs files, it sounds like it's not actually the *driver* that needs > > this; it's something in userspace? > We have use cases to change the link speed and width on the fly. > One is EMI check and other is power saving. Some battery backed > applications have to switch PCIe link from higher GEN to GEN1 and > width to x1. During the cases like external power supply got > disconnected or broken. Once external power supply is connected then > switch PCIe link to higher GEN and width. That sounds plausible, but of course nothing there is specific to the Intel Gateway, so we should implement this generically so it would work on all hardware. I'm not sure what the interface should look like -- should it be a low-level interface as you propose where userspace would have to identify each link of interest, or is there some system-wide power/performance knob that could tune all links? Cc'd Rafael and linux-pm in case they have ideas. Bjorn
Re: [PATCH v4 3/3] pci: intel: Add sysfs attributes to configure pcie link
Hi Bjorn Helgaas,
On 10/22/2019 1:18 AM, Bjorn Helgaas wrote:
On Mon, Oct 21, 2019 at 02:38:50PM +0100, Andrew Murray wrote:
On Mon, Oct 21, 2019 at 02:39:20PM +0800, Dilip Kota wrote:
PCIe RC driver on Intel Gateway SoCs have a requirement
of changing link width and speed on the fly.
Please add more details about why this is needed. Since you're adding
sysfs files, it sounds like it's not actually the *driver* that needs
this; it's something in userspace?
We have use cases to change the link speed and width on the fly.
One is EMI check and other is power saving.
Some battery backed applications have to switch PCIe link from higher
GEN to GEN1 and width to x1. During the cases like
external power supply got disconnected or broken. Once external power
supply is connected then switch PCIe link to higher GEN and width.
The normal scenario is that the hardware negotiates link widths and
speeds without any software involvement (PCIe r5.0, sec 1.2).
If this is to work around hardware defects, we should try to do that
inside the kernel because we can't expect userspace to do it reliably.
As Andrew points out below, this all sounds like it should be generic
rather than Intel-specific.
So add the sysfs attributes to show and store the link
properties.
Add the respective link resize function in pcie DesignWare
framework so that Intel PCIe driver can use during link
width configuration on the fly.
...
+static ssize_t pcie_link_status_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ u32 reg, width, gen;
+
+ reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
+ width = FIELD_GET(PCI_EXP_LNKSTA_NLW, reg >> 16);
+ gen = FIELD_GET(PCI_EXP_LNKSTA_CLS, reg >> 16);
+
+ if (gen > lpp->max_speed)
+ return -EINVAL;
+
+ return sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
+ width, pcie_link_gen_to_str(gen));
+}
+static DEVICE_ATTR_RO(pcie_link_status);
We already have generic current_link_speed and current_link_width
files.
Thanks for pointing it. I will remove the pcie_link_status.
Regards,
Dilip
+static ssize_t pcie_speed_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, );
+ if (ret)
+ return ret;
+
+ if (val > lpp->max_speed)
+ return -EINVAL;
+
+ lpp->link_gen = val;
+ intel_pcie_max_speed_setup(lpp);
+ dw_pcie_link_speed_change(>pci, false);
+ dw_pcie_link_speed_change(>pci, true);
+
+ return len;
+}
+static DEVICE_ATTR_WO(pcie_speed);
+
+/*
+ * Link width change on the fly is not always successful.
+ * It also depends on the partner.
+ */
+static ssize_t pcie_width_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ lpp = dev_get_drvdata(dev);
+
+ ret = kstrtoul(buf, 10, );
+ if (ret)
+ return ret;
+
+ if (val > lpp->max_width)
+ return -EINVAL;
+
+ /* HW auto bandwidth negotiation must be enabled */
+ pcie_rc_cfg_wr_mask(lpp, PCI_EXP_LNKCTL_HAWD, 0,
+ PCIE_CAP_OFST + PCI_EXP_LNKCTL);
+ dw_pcie_link_width_resize(>pci, val);
+
+ return len;
+}
+static DEVICE_ATTR_WO(pcie_width);
+
+static struct attribute *pcie_cfg_attrs[] = {
+ _attr_pcie_link_status.attr,
+ _attr_pcie_speed.attr,
+ _attr_pcie_width.attr,
+ NULL,
+};
Is there a reason that these are limited only to the Intel driver and
not the wider set of DWC drivers?
Is there anything specific here about the Intel GW driver?
Re: [PATCH v4 3/3] pci: intel: Add sysfs attributes to configure pcie link
Hi Andrew Murray,
On 10/21/2019 9:38 PM, Andrew Murray wrote:
On Mon, Oct 21, 2019 at 02:39:20PM +0800, Dilip Kota wrote:
PCIe RC driver on Intel Gateway SoCs have a requirement
of changing link width and speed on the fly.
So add the sysfs attributes to show and store the link
properties.
Add the respective link resize function in pcie DesignWare
framework so that Intel PCIe driver can use during link
width configuration on the fly.
Signed-off-by: Dilip Kota
---
drivers/pci/controller/dwc/pcie-designware.c | 9 +++
drivers/pci/controller/dwc/pcie-designware.h | 3 +
drivers/pci/controller/dwc/pcie-intel-gw.c | 112 ++-
3 files changed, 123 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c
b/drivers/pci/controller/dwc/pcie-designware.c
index 4c391bfd681a..662fdcb4f2d6 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -474,6 +474,15 @@ int dw_pcie_link_up(struct dw_pcie *pci)
(!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
}
+void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width)
+{
+ u32 val;
+
+ val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
+ val &= ~(PORT_MLTI_LNK_WDTH_CHNG | PORT_MLTI_LNK_WDTH);
+ val |= PORT_MLTI_LNK_WDTH_CHNG | lane_width;
+ dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
+}
void dw_pcie_upconfig_setup(struct dw_pcie *pci)
{
diff --git a/drivers/pci/controller/dwc/pcie-designware.h
b/drivers/pci/controller/dwc/pcie-designware.h
index 3beac10e4a4c..fcf0442341fd 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -67,6 +67,8 @@
#define PCIE_MSI_INTR0_STATUS 0x830
#define PCIE_PORT_MULTI_LANE_CTRL 0x8C0
+#define PORT_MLTI_LNK_WDTH GENMASK(5, 0)
+#define PORT_MLTI_LNK_WDTH_CHNGBIT(6)
#define PORT_MLTI_UPCFG_SUPPORT BIT(7)
#define PCIE_ATU_VIEWPORT 0x900
@@ -282,6 +284,7 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg,
size_t size, u32 val);
u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size);
void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
int dw_pcie_link_up(struct dw_pcie *pci);
+void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width);
void dw_pcie_upconfig_setup(struct dw_pcie *pci);
void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable);
void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts);
diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 9142c70db808..b9be0921671d 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -146,6 +146,22 @@ static void intel_pcie_ltssm_disable(struct
intel_pcie_port *lpp)
pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE, 0, PCIE_APP_CCR);
}
+static const char *pcie_link_gen_to_str(int gen)
+{
+ switch (gen) {
+ case PCIE_LINK_SPEED_GEN1:
+ return "2.5";
+ case PCIE_LINK_SPEED_GEN2:
+ return "5.0";
+ case PCIE_LINK_SPEED_GEN3:
+ return "8.0";
+ case PCIE_LINK_SPEED_GEN4:
+ return "16.0";
+ default:
+ return "???";
+ }
+}
+
static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
{
u32 val;
@@ -444,6 +460,91 @@ static int intel_pcie_host_setup(struct intel_pcie_port
*lpp)
return ret;
}
+static ssize_t pcie_link_status_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ u32 reg, width, gen;
+
+ reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
+ width = FIELD_GET(PCI_EXP_LNKSTA_NLW, reg >> 16);
+ gen = FIELD_GET(PCI_EXP_LNKSTA_CLS, reg >> 16);
+
+ if (gen > lpp->max_speed)
+ return -EINVAL;
+
+ return sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
+ width, pcie_link_gen_to_str(gen));
+}
+static DEVICE_ATTR_RO(pcie_link_status);
+
+static ssize_t pcie_speed_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, );
+ if (ret)
+ return ret;
+
+ if (val > lpp->max_speed)
+ return -EINVAL;
+
+ lpp->link_gen = val;
+ intel_pcie_max_speed_setup(lpp);
+ dw_pcie_link_speed_change(>pci, false);
+ dw_pcie_link_speed_change(>pci, true);
+
+ return len;
+}
+static DEVICE_ATTR_WO(pcie_speed);
+
+/*
+ * Link width change on the fly is not always successful.
Re: [PATCH v4 3/3] pci: intel: Add sysfs attributes to configure pcie link
On Mon, Oct 21, 2019 at 02:38:50PM +0100, Andrew Murray wrote:
> On Mon, Oct 21, 2019 at 02:39:20PM +0800, Dilip Kota wrote:
> > PCIe RC driver on Intel Gateway SoCs have a requirement
> > of changing link width and speed on the fly.
Please add more details about why this is needed. Since you're adding
sysfs files, it sounds like it's not actually the *driver* that needs
this; it's something in userspace?
The normal scenario is that the hardware negotiates link widths and
speeds without any software involvement (PCIe r5.0, sec 1.2).
If this is to work around hardware defects, we should try to do that
inside the kernel because we can't expect userspace to do it reliably.
As Andrew points out below, this all sounds like it should be generic
rather than Intel-specific.
> > So add the sysfs attributes to show and store the link
> > properties.
> > Add the respective link resize function in pcie DesignWare
> > framework so that Intel PCIe driver can use during link
> > width configuration on the fly.
> > ...
> > +static ssize_t pcie_link_status_show(struct device *dev,
> > +struct device_attribute *attr, char *buf)
> > +{
> > + struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> > + u32 reg, width, gen;
> > +
> > + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> > + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, reg >> 16);
> > + gen = FIELD_GET(PCI_EXP_LNKSTA_CLS, reg >> 16);
> > +
> > + if (gen > lpp->max_speed)
> > + return -EINVAL;
> > +
> > + return sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
> > + width, pcie_link_gen_to_str(gen));
> > +}
> > +static DEVICE_ATTR_RO(pcie_link_status);
We already have generic current_link_speed and current_link_width
files.
> > +static ssize_t pcie_speed_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t len)
> > +{
> > + struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> > + unsigned long val;
> > + int ret;
> > +
> > + ret = kstrtoul(buf, 10, );
> > + if (ret)
> > + return ret;
> > +
> > + if (val > lpp->max_speed)
> > + return -EINVAL;
> > +
> > + lpp->link_gen = val;
> > + intel_pcie_max_speed_setup(lpp);
> > + dw_pcie_link_speed_change(>pci, false);
> > + dw_pcie_link_speed_change(>pci, true);
> > +
> > + return len;
> > +}
> > +static DEVICE_ATTR_WO(pcie_speed);
> > +
> > +/*
> > + * Link width change on the fly is not always successful.
> > + * It also depends on the partner.
> > + */
> > +static ssize_t pcie_width_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t len)
> > +{
> > + struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> > + unsigned long val;
> > + int ret;
> > +
> > + lpp = dev_get_drvdata(dev);
> > +
> > + ret = kstrtoul(buf, 10, );
> > + if (ret)
> > + return ret;
> > +
> > + if (val > lpp->max_width)
> > + return -EINVAL;
> > +
> > + /* HW auto bandwidth negotiation must be enabled */
> > + pcie_rc_cfg_wr_mask(lpp, PCI_EXP_LNKCTL_HAWD, 0,
> > + PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> > + dw_pcie_link_width_resize(>pci, val);
> > +
> > + return len;
> > +}
> > +static DEVICE_ATTR_WO(pcie_width);
> > +
> > +static struct attribute *pcie_cfg_attrs[] = {
> > + _attr_pcie_link_status.attr,
> > + _attr_pcie_speed.attr,
> > + _attr_pcie_width.attr,
> > + NULL,
> > +};
>
> Is there a reason that these are limited only to the Intel driver and
> not the wider set of DWC drivers?
>
> Is there anything specific here about the Intel GW driver?
Re: [PATCH v4 3/3] pci: intel: Add sysfs attributes to configure pcie link
On Mon, Oct 21, 2019 at 02:39:20PM +0800, Dilip Kota wrote:
> PCIe RC driver on Intel Gateway SoCs have a requirement
> of changing link width and speed on the fly.
> So add the sysfs attributes to show and store the link
> properties.
> Add the respective link resize function in pcie DesignWare
> framework so that Intel PCIe driver can use during link
> width configuration on the fly.
>
> Signed-off-by: Dilip Kota
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 9 +++
> drivers/pci/controller/dwc/pcie-designware.h | 3 +
> drivers/pci/controller/dwc/pcie-intel-gw.c | 112
> ++-
> 3 files changed, 123 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> b/drivers/pci/controller/dwc/pcie-designware.c
> index 4c391bfd681a..662fdcb4f2d6 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -474,6 +474,15 @@ int dw_pcie_link_up(struct dw_pcie *pci)
> (!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
> }
>
> +void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width)
> +{
> + u32 val;
> +
> + val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
> + val &= ~(PORT_MLTI_LNK_WDTH_CHNG | PORT_MLTI_LNK_WDTH);
> + val |= PORT_MLTI_LNK_WDTH_CHNG | lane_width;
> + dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
> +}
>
> void dw_pcie_upconfig_setup(struct dw_pcie *pci)
> {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> b/drivers/pci/controller/dwc/pcie-designware.h
> index 3beac10e4a4c..fcf0442341fd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -67,6 +67,8 @@
> #define PCIE_MSI_INTR0_STATUS0x830
>
> #define PCIE_PORT_MULTI_LANE_CTRL0x8C0
> +#define PORT_MLTI_LNK_WDTH GENMASK(5, 0)
> +#define PORT_MLTI_LNK_WDTH_CHNG BIT(6)
> #define PORT_MLTI_UPCFG_SUPPORT BIT(7)
>
> #define PCIE_ATU_VIEWPORT0x900
> @@ -282,6 +284,7 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg,
> size_t size, u32 val);
> u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size);
> void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
> int dw_pcie_link_up(struct dw_pcie *pci);
> +void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width);
> void dw_pcie_upconfig_setup(struct dw_pcie *pci);
> void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable);
> void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts);
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
> b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 9142c70db808..b9be0921671d 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -146,6 +146,22 @@ static void intel_pcie_ltssm_disable(struct
> intel_pcie_port *lpp)
> pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE, 0, PCIE_APP_CCR);
> }
>
> +static const char *pcie_link_gen_to_str(int gen)
> +{
> + switch (gen) {
> + case PCIE_LINK_SPEED_GEN1:
> + return "2.5";
> + case PCIE_LINK_SPEED_GEN2:
> + return "5.0";
> + case PCIE_LINK_SPEED_GEN3:
> + return "8.0";
> + case PCIE_LINK_SPEED_GEN4:
> + return "16.0";
> + default:
> + return "???";
> + }
> +}
> +
> static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
> {
> u32 val;
> @@ -444,6 +460,91 @@ static int intel_pcie_host_setup(struct intel_pcie_port
> *lpp)
> return ret;
> }
>
> +static ssize_t pcie_link_status_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> + u32 reg, width, gen;
> +
> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, reg >> 16);
> + gen = FIELD_GET(PCI_EXP_LNKSTA_CLS, reg >> 16);
> +
> + if (gen > lpp->max_speed)
> + return -EINVAL;
> +
> + return sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
> +width, pcie_link_gen_to_str(gen));
> +}
> +static DEVICE_ATTR_RO(pcie_link_status);
> +
> +static ssize_t pcie_speed_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, );
> + if (ret)
> + return ret;
> +
> + if (val > lpp->max_speed)
> + return -EINVAL;
> +
> + lpp->link_gen = val;
> + intel_pcie_max_speed_setup(lpp);
> + dw_pcie_link_speed_change(>pci, false);
> + dw_pcie_link_speed_change(>pci, true);
> +
> + return len;
> +}
>
Re: [PATCH v4 3/3] pci: intel: Add sysfs attributes to configure pcie link
Hi Gustavo Pimentel,
On 10/21/2019 4:40 PM, Gustavo Pimentel wrote:
On Mon, Oct 21, 2019 at 7:39:20, Dilip Kota
wrote:
PCIe RC driver on Intel Gateway SoCs have a requirement
of changing link width and speed on the fly.
So add the sysfs attributes to show and store the link
properties.
Add the respective link resize function in pcie DesignWare
framework so that Intel PCIe driver can use during link
width configuration on the fly.
Signed-off-by: Dilip Kota
---
drivers/pci/controller/dwc/pcie-designware.c | 9 +++
drivers/pci/controller/dwc/pcie-designware.h | 3 +
drivers/pci/controller/dwc/pcie-intel-gw.c | 112 ++-
3 files changed, 123 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c
b/drivers/pci/controller/dwc/pcie-designware.c
index 4c391bfd681a..662fdcb4f2d6 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -474,6 +474,15 @@ int dw_pcie_link_up(struct dw_pcie *pci)
(!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
}
+void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width)
+{
+ u32 val;
+
+ val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
+ val &= ~(PORT_MLTI_LNK_WDTH_CHNG | PORT_MLTI_LNK_WDTH);
+ val |= PORT_MLTI_LNK_WDTH_CHNG | lane_width;
+ dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
+}
void dw_pcie_upconfig_setup(struct dw_pcie *pci)
{
diff --git a/drivers/pci/controller/dwc/pcie-designware.h
b/drivers/pci/controller/dwc/pcie-designware.h
index 3beac10e4a4c..fcf0442341fd 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -67,6 +67,8 @@
#define PCIE_MSI_INTR0_STATUS 0x830
#define PCIE_PORT_MULTI_LANE_CTRL 0x8C0
+#define PORT_MLTI_LNK_WDTH GENMASK(5, 0)
+#define PORT_MLTI_LNK_WDTH_CHNGBIT(6)
#define PORT_MLTI_UPCFG_SUPPORT BIT(7)
#define PCIE_ATU_VIEWPORT 0x900
@@ -282,6 +284,7 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg,
size_t size, u32 val);
u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size);
void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
int dw_pcie_link_up(struct dw_pcie *pci);
+void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width);
void dw_pcie_upconfig_setup(struct dw_pcie *pci);
void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable);
void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts);
diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 9142c70db808..b9be0921671d 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -146,6 +146,22 @@ static void intel_pcie_ltssm_disable(struct
intel_pcie_port *lpp)
pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE, 0, PCIE_APP_CCR);
}
+static const char *pcie_link_gen_to_str(int gen)
+{
+ switch (gen) {
+ case PCIE_LINK_SPEED_GEN1:
+ return "2.5";
+ case PCIE_LINK_SPEED_GEN2:
+ return "5.0";
+ case PCIE_LINK_SPEED_GEN3:
+ return "8.0";
+ case PCIE_LINK_SPEED_GEN4:
+ return "16.0";
+ default:
+ return "???";
+ }
+}
+
static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
{
u32 val;
@@ -444,6 +460,91 @@ static int intel_pcie_host_setup(struct intel_pcie_port
*lpp)
return ret;
}
+static ssize_t pcie_link_status_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ u32 reg, width, gen;
+
+ reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
+ width = FIELD_GET(PCI_EXP_LNKSTA_NLW, reg >> 16);
+ gen = FIELD_GET(PCI_EXP_LNKSTA_CLS, reg >> 16);
+
+ if (gen > lpp->max_speed)
+ return -EINVAL;
+
+ return sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
+ width, pcie_link_gen_to_str(gen));
+}
+static DEVICE_ATTR_RO(pcie_link_status);
Dilip please check pci.h there are there already enums and strings
relatively to PCIe speed and width, that you can use.
Yes i can see a global array "pcie_link_speed[]" and a macro
PCIE_SPEED2STR[].
I will update the driver.
Whereas width enum, it is not required here as directly storing the
register value.
Thanks for pointing it.
Regards,
Dilip
+
+static ssize_t pcie_speed_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, );
+ if (ret)
+ return ret;
+
RE: [PATCH v4 3/3] pci: intel: Add sysfs attributes to configure pcie link
On Mon, Oct 21, 2019 at 7:39:20, Dilip Kota
wrote:
> PCIe RC driver on Intel Gateway SoCs have a requirement
> of changing link width and speed on the fly.
> So add the sysfs attributes to show and store the link
> properties.
> Add the respective link resize function in pcie DesignWare
> framework so that Intel PCIe driver can use during link
> width configuration on the fly.
>
> Signed-off-by: Dilip Kota
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 9 +++
> drivers/pci/controller/dwc/pcie-designware.h | 3 +
> drivers/pci/controller/dwc/pcie-intel-gw.c | 112
> ++-
> 3 files changed, 123 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> b/drivers/pci/controller/dwc/pcie-designware.c
> index 4c391bfd681a..662fdcb4f2d6 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -474,6 +474,15 @@ int dw_pcie_link_up(struct dw_pcie *pci)
> (!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
> }
>
> +void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width)
> +{
> + u32 val;
> +
> + val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
> + val &= ~(PORT_MLTI_LNK_WDTH_CHNG | PORT_MLTI_LNK_WDTH);
> + val |= PORT_MLTI_LNK_WDTH_CHNG | lane_width;
> + dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
> +}
>
> void dw_pcie_upconfig_setup(struct dw_pcie *pci)
> {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> b/drivers/pci/controller/dwc/pcie-designware.h
> index 3beac10e4a4c..fcf0442341fd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -67,6 +67,8 @@
> #define PCIE_MSI_INTR0_STATUS0x830
>
> #define PCIE_PORT_MULTI_LANE_CTRL0x8C0
> +#define PORT_MLTI_LNK_WDTH GENMASK(5, 0)
> +#define PORT_MLTI_LNK_WDTH_CHNG BIT(6)
> #define PORT_MLTI_UPCFG_SUPPORT BIT(7)
>
> #define PCIE_ATU_VIEWPORT0x900
> @@ -282,6 +284,7 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg,
> size_t size, u32 val);
> u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size);
> void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
> int dw_pcie_link_up(struct dw_pcie *pci);
> +void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width);
> void dw_pcie_upconfig_setup(struct dw_pcie *pci);
> void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable);
> void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts);
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
> b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 9142c70db808..b9be0921671d 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -146,6 +146,22 @@ static void intel_pcie_ltssm_disable(struct
> intel_pcie_port *lpp)
> pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE, 0, PCIE_APP_CCR);
> }
>
> +static const char *pcie_link_gen_to_str(int gen)
> +{
> + switch (gen) {
> + case PCIE_LINK_SPEED_GEN1:
> + return "2.5";
> + case PCIE_LINK_SPEED_GEN2:
> + return "5.0";
> + case PCIE_LINK_SPEED_GEN3:
> + return "8.0";
> + case PCIE_LINK_SPEED_GEN4:
> + return "16.0";
> + default:
> + return "???";
> + }
> +}
> +
> static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
> {
> u32 val;
> @@ -444,6 +460,91 @@ static int intel_pcie_host_setup(struct intel_pcie_port
> *lpp)
> return ret;
> }
>
> +static ssize_t pcie_link_status_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> + u32 reg, width, gen;
> +
> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, reg >> 16);
> + gen = FIELD_GET(PCI_EXP_LNKSTA_CLS, reg >> 16);
> +
> + if (gen > lpp->max_speed)
> + return -EINVAL;
> +
> + return sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
> +width, pcie_link_gen_to_str(gen));
> +}
> +static DEVICE_ATTR_RO(pcie_link_status);
Dilip please check pci.h there are there already enums and strings
relatively to PCIe speed and width, that you can use.
> +
> +static ssize_t pcie_speed_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> + unsigned long val;
> + int ret;
> +
> + ret = kstrtoul(buf, 10, );
> + if (ret)
> + return ret;
> +
> + if (val > lpp->max_speed)
> + return -EINVAL;
> +
> + lpp->link_gen = val;
> + intel_pcie_max_speed_setup(lpp);
> +
[PATCH v4 3/3] pci: intel: Add sysfs attributes to configure pcie link
PCIe RC driver on Intel Gateway SoCs have a requirement
of changing link width and speed on the fly.
So add the sysfs attributes to show and store the link
properties.
Add the respective link resize function in pcie DesignWare
framework so that Intel PCIe driver can use during link
width configuration on the fly.
Signed-off-by: Dilip Kota
---
drivers/pci/controller/dwc/pcie-designware.c | 9 +++
drivers/pci/controller/dwc/pcie-designware.h | 3 +
drivers/pci/controller/dwc/pcie-intel-gw.c | 112 ++-
3 files changed, 123 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c
b/drivers/pci/controller/dwc/pcie-designware.c
index 4c391bfd681a..662fdcb4f2d6 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -474,6 +474,15 @@ int dw_pcie_link_up(struct dw_pcie *pci)
(!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
}
+void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width)
+{
+ u32 val;
+
+ val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
+ val &= ~(PORT_MLTI_LNK_WDTH_CHNG | PORT_MLTI_LNK_WDTH);
+ val |= PORT_MLTI_LNK_WDTH_CHNG | lane_width;
+ dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
+}
void dw_pcie_upconfig_setup(struct dw_pcie *pci)
{
diff --git a/drivers/pci/controller/dwc/pcie-designware.h
b/drivers/pci/controller/dwc/pcie-designware.h
index 3beac10e4a4c..fcf0442341fd 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -67,6 +67,8 @@
#define PCIE_MSI_INTR0_STATUS 0x830
#define PCIE_PORT_MULTI_LANE_CTRL 0x8C0
+#define PORT_MLTI_LNK_WDTH GENMASK(5, 0)
+#define PORT_MLTI_LNK_WDTH_CHNGBIT(6)
#define PORT_MLTI_UPCFG_SUPPORTBIT(7)
#define PCIE_ATU_VIEWPORT 0x900
@@ -282,6 +284,7 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg,
size_t size, u32 val);
u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size);
void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
int dw_pcie_link_up(struct dw_pcie *pci);
+void dw_pcie_link_width_resize(struct dw_pcie *pci, u32 lane_width);
void dw_pcie_upconfig_setup(struct dw_pcie *pci);
void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable);
void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts);
diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 9142c70db808..b9be0921671d 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -146,6 +146,22 @@ static void intel_pcie_ltssm_disable(struct
intel_pcie_port *lpp)
pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE, 0, PCIE_APP_CCR);
}
+static const char *pcie_link_gen_to_str(int gen)
+{
+ switch (gen) {
+ case PCIE_LINK_SPEED_GEN1:
+ return "2.5";
+ case PCIE_LINK_SPEED_GEN2:
+ return "5.0";
+ case PCIE_LINK_SPEED_GEN3:
+ return "8.0";
+ case PCIE_LINK_SPEED_GEN4:
+ return "16.0";
+ default:
+ return "???";
+ }
+}
+
static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
{
u32 val;
@@ -444,6 +460,91 @@ static int intel_pcie_host_setup(struct intel_pcie_port
*lpp)
return ret;
}
+static ssize_t pcie_link_status_show(struct device *dev,
+struct device_attribute *attr, char *buf)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ u32 reg, width, gen;
+
+ reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
+ width = FIELD_GET(PCI_EXP_LNKSTA_NLW, reg >> 16);
+ gen = FIELD_GET(PCI_EXP_LNKSTA_CLS, reg >> 16);
+
+ if (gen > lpp->max_speed)
+ return -EINVAL;
+
+ return sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
+ width, pcie_link_gen_to_str(gen));
+}
+static DEVICE_ATTR_RO(pcie_link_status);
+
+static ssize_t pcie_speed_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ ret = kstrtoul(buf, 10, );
+ if (ret)
+ return ret;
+
+ if (val > lpp->max_speed)
+ return -EINVAL;
+
+ lpp->link_gen = val;
+ intel_pcie_max_speed_setup(lpp);
+ dw_pcie_link_speed_change(>pci, false);
+ dw_pcie_link_speed_change(>pci, true);
+
+ return len;
+}
+static DEVICE_ATTR_WO(pcie_speed);
+
+/*
+ * Link width change on the fly is not always successful.
+ * It also depends on the partner.
+ */
+static ssize_t pcie_width_store(struct device *dev,
+ struct
