* [PATCH] pci: tegra: correctly program PADS_REFCLK registers
@ 2016-06-21 18:46 Stephen Warren
2016-06-22 12:57 ` Thierry Reding
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Warren @ 2016-06-21 18:46 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Thierry Reding, linux-tegra, linux-pci, Stephen Warren
From: Stephen Warren <swarren@nvidia.com>
The value that should be programmed into the PADS_REFCLK register varies
per SoC. Fix the Tegra PCIe driver to program the correct values. Future
SoCs will require different values in cfg0/1, so the two values are stored
separately in the per-SoC data structures.
For reference, the values are all documented in NV bug 1771116 comment 20.
Rhe ASIC team has validated all these values, except for the Tegra20 value
which is simply left unchanged in this patch.
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
drivers/pci/host/pci-tegra.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index c388468c202a..74887fedc3d4 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -228,15 +228,6 @@
#define PADS_REFCLK_CFG_PREDI_SHIFT 8 /* 11:8 */
#define PADS_REFCLK_CFG_DRVI_SHIFT 12 /* 15:12 */
-/* Default value provided by HW engineering is 0xfa5c */
-#define PADS_REFCLK_CFG_VALUE \
- ( \
- (0x17 << PADS_REFCLK_CFG_TERM_SHIFT) | \
- (0 << PADS_REFCLK_CFG_E_TERM_SHIFT) | \
- (0xa << PADS_REFCLK_CFG_PREDI_SHIFT) | \
- (0xf << PADS_REFCLK_CFG_DRVI_SHIFT) \
- )
-
struct tegra_msi {
struct msi_controller chip;
DECLARE_BITMAP(used, INT_PCI_MSI_NR);
@@ -252,6 +243,8 @@ struct tegra_pcie_soc_data {
unsigned int msi_base_shift;
u32 pads_pll_ctl;
u32 tx_ref_sel;
+ u32 pads_refclk_cfg0;
+ u32 pads_refclk_cfg1;
bool has_pex_clkreq_en;
bool has_pex_bias_ctrl;
bool has_intr_prsnt_sense;
@@ -839,10 +832,9 @@ static int tegra_pcie_phy_enable(struct tegra_pcie *pcie)
pads_writel(pcie, value, soc->pads_pll_ctl);
/* Configure the reference clock driver */
- value = PADS_REFCLK_CFG_VALUE | (PADS_REFCLK_CFG_VALUE << 16);
- pads_writel(pcie, value, PADS_REFCLK_CFG0);
+ pads_writel(pcie, soc->pads_refclk_cfg0, PADS_REFCLK_CFG0);
if (soc->num_ports > 2)
- pads_writel(pcie, PADS_REFCLK_CFG_VALUE, PADS_REFCLK_CFG1);
+ pads_writel(pcie, soc->pads_refclk_cfg1, PADS_REFCLK_CFG1);
/* wait for the PLL to lock */
err = tegra_pcie_pll_wait(pcie, 500);
@@ -2078,6 +2070,7 @@ static const struct tegra_pcie_soc_data tegra20_pcie_data = {
.msi_base_shift = 0,
.pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
+ .pads_refclk_cfg0 = 0xfa5cfa5c,
.has_pex_clkreq_en = false,
.has_pex_bias_ctrl = false,
.has_intr_prsnt_sense = false,
@@ -2090,6 +2083,8 @@ static const struct tegra_pcie_soc_data tegra30_pcie_data = {
.msi_base_shift = 8,
.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+ .pads_refclk_cfg0 = 0xfa5cfa5c,
+ .pads_refclk_cfg1 = 0xfa5cfa5c,
.has_pex_clkreq_en = true,
.has_pex_bias_ctrl = true,
.has_intr_prsnt_sense = true,
@@ -2102,6 +2097,7 @@ static const struct tegra_pcie_soc_data tegra124_pcie_data = {
.msi_base_shift = 8,
.pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
.tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
+ .pads_refclk_cfg0 = 0x44ac44ac,
.has_pex_clkreq_en = true,
.has_pex_bias_ctrl = true,
.has_intr_prsnt_sense = true,
--
2.9.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] pci: tegra: correctly program PADS_REFCLK registers
2016-06-21 18:46 [PATCH] pci: tegra: correctly program PADS_REFCLK registers Stephen Warren
@ 2016-06-22 12:57 ` Thierry Reding
2016-06-22 15:34 ` Stephen Warren
0 siblings, 1 reply; 7+ messages in thread
From: Thierry Reding @ 2016-06-22 12:57 UTC (permalink / raw)
To: Stephen Warren; +Cc: Bjorn Helgaas, linux-tegra, linux-pci, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 3659 bytes --]
On Tue, Jun 21, 2016 at 12:46:40PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> The value that should be programmed into the PADS_REFCLK register varies
> per SoC. Fix the Tegra PCIe driver to program the correct values. Future
> SoCs will require different values in cfg0/1, so the two values are stored
> separately in the per-SoC data structures.
>
> For reference, the values are all documented in NV bug 1771116 comment 20.
> Rhe ASIC team has validated all these values, except for the Tegra20 value
> which is simply left unchanged in this patch.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> drivers/pci/host/pci-tegra.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index c388468c202a..74887fedc3d4 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -228,15 +228,6 @@
> #define PADS_REFCLK_CFG_PREDI_SHIFT 8 /* 11:8 */
> #define PADS_REFCLK_CFG_DRVI_SHIFT 12 /* 15:12 */
>
> -/* Default value provided by HW engineering is 0xfa5c */
> -#define PADS_REFCLK_CFG_VALUE \
> - ( \
> - (0x17 << PADS_REFCLK_CFG_TERM_SHIFT) | \
> - (0 << PADS_REFCLK_CFG_E_TERM_SHIFT) | \
> - (0xa << PADS_REFCLK_CFG_PREDI_SHIFT) | \
> - (0xf << PADS_REFCLK_CFG_DRVI_SHIFT) \
> - )
> -
> struct tegra_msi {
> struct msi_controller chip;
> DECLARE_BITMAP(used, INT_PCI_MSI_NR);
> @@ -252,6 +243,8 @@ struct tegra_pcie_soc_data {
> unsigned int msi_base_shift;
> u32 pads_pll_ctl;
> u32 tx_ref_sel;
> + u32 pads_refclk_cfg0;
> + u32 pads_refclk_cfg1;
> bool has_pex_clkreq_en;
> bool has_pex_bias_ctrl;
> bool has_intr_prsnt_sense;
> @@ -839,10 +832,9 @@ static int tegra_pcie_phy_enable(struct tegra_pcie *pcie)
> pads_writel(pcie, value, soc->pads_pll_ctl);
>
> /* Configure the reference clock driver */
> - value = PADS_REFCLK_CFG_VALUE | (PADS_REFCLK_CFG_VALUE << 16);
> - pads_writel(pcie, value, PADS_REFCLK_CFG0);
> + pads_writel(pcie, soc->pads_refclk_cfg0, PADS_REFCLK_CFG0);
> if (soc->num_ports > 2)
> - pads_writel(pcie, PADS_REFCLK_CFG_VALUE, PADS_REFCLK_CFG1);
> + pads_writel(pcie, soc->pads_refclk_cfg1, PADS_REFCLK_CFG1);
>
> /* wait for the PLL to lock */
> err = tegra_pcie_pll_wait(pcie, 500);
> @@ -2078,6 +2070,7 @@ static const struct tegra_pcie_soc_data tegra20_pcie_data = {
> .msi_base_shift = 0,
> .pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
> + .pads_refclk_cfg0 = 0xfa5cfa5c,
> .has_pex_clkreq_en = false,
> .has_pex_bias_ctrl = false,
> .has_intr_prsnt_sense = false,
> @@ -2090,6 +2083,8 @@ static const struct tegra_pcie_soc_data tegra30_pcie_data = {
> .msi_base_shift = 8,
> .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> + .pads_refclk_cfg0 = 0xfa5cfa5c,
> + .pads_refclk_cfg1 = 0xfa5cfa5c,
> .has_pex_clkreq_en = true,
> .has_pex_bias_ctrl = true,
> .has_intr_prsnt_sense = true,
> @@ -2102,6 +2097,7 @@ static const struct tegra_pcie_soc_data tegra124_pcie_data = {
> .msi_base_shift = 8,
> .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> + .pads_refclk_cfg0 = 0x44ac44ac,
> .has_pex_clkreq_en = true,
> .has_pex_bias_ctrl = true,
> .has_intr_prsnt_sense = true,
I think it'd be nice to have these decoded into their individual fields,
to reduce the magic. We already define the register fields, so it seems
sensible to use them.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] pci: tegra: correctly program PADS_REFCLK registers
2016-06-22 12:57 ` Thierry Reding
@ 2016-06-22 15:34 ` Stephen Warren
2016-06-23 8:44 ` Thierry Reding
0 siblings, 1 reply; 7+ messages in thread
From: Stephen Warren @ 2016-06-22 15:34 UTC (permalink / raw)
To: Thierry Reding; +Cc: Bjorn Helgaas, linux-tegra, linux-pci, Stephen Warren
On 06/22/2016 06:57 AM, Thierry Reding wrote:
> On Tue, Jun 21, 2016 at 12:46:40PM -0600, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> The value that should be programmed into the PADS_REFCLK register varies
>> per SoC. Fix the Tegra PCIe driver to program the correct values. Future
>> SoCs will require different values in cfg0/1, so the two values are stored
>> separately in the per-SoC data structures.
>>
>> For reference, the values are all documented in NV bug 1771116 comment 20.
>> Rhe ASIC team has validated all these values, except for the Tegra20 value
>> which is simply left unchanged in this patch.
>> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>> @@ -2078,6 +2070,7 @@ static const struct tegra_pcie_soc_data tegra20_pcie_data = {
>> .msi_base_shift = 0,
>> .pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
>> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
>> + .pads_refclk_cfg0 = 0xfa5cfa5c,
>> .has_pex_clkreq_en = false,
>> .has_pex_bias_ctrl = false,
>> .has_intr_prsnt_sense = false,
>> @@ -2090,6 +2083,8 @@ static const struct tegra_pcie_soc_data tegra30_pcie_data = {
>> .msi_base_shift = 8,
>> .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
>> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
>> + .pads_refclk_cfg0 = 0xfa5cfa5c,
>> + .pads_refclk_cfg1 = 0xfa5cfa5c,
>> .has_pex_clkreq_en = true,
>> .has_pex_bias_ctrl = true,
>> .has_intr_prsnt_sense = true,
>> @@ -2102,6 +2097,7 @@ static const struct tegra_pcie_soc_data tegra124_pcie_data = {
>> .msi_base_shift = 8,
>> .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
>> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
>> + .pads_refclk_cfg0 = 0x44ac44ac,
>> .has_pex_clkreq_en = true,
>> .has_pex_bias_ctrl = true,
>> .has_intr_prsnt_sense = true,
>
> I think it'd be nice to have these decoded into their individual fields,
> to reduce the magic. We already define the register fields, so it seems
> sensible to use them.
I did consider that. However, the specification from the ASIC team is
always the raw values. Decoding them into bitfields is only going to
make it harder to verify whether the correct values are present (since
the reader has to manually expand the math), and introduce the
possibility of errors during the expansion process. I think using raw
numbers is better in this case.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] pci: tegra: correctly program PADS_REFCLK registers
2016-06-22 15:34 ` Stephen Warren
@ 2016-06-23 8:44 ` Thierry Reding
2016-06-30 13:20 ` Thierry Reding
0 siblings, 1 reply; 7+ messages in thread
From: Thierry Reding @ 2016-06-23 8:44 UTC (permalink / raw)
To: Stephen Warren; +Cc: Bjorn Helgaas, linux-tegra, linux-pci, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]
On Wed, Jun 22, 2016 at 09:34:21AM -0600, Stephen Warren wrote:
> On 06/22/2016 06:57 AM, Thierry Reding wrote:
> > On Tue, Jun 21, 2016 at 12:46:40PM -0600, Stephen Warren wrote:
> > > From: Stephen Warren <swarren@nvidia.com>
> > >
> > > The value that should be programmed into the PADS_REFCLK register varies
> > > per SoC. Fix the Tegra PCIe driver to program the correct values. Future
> > > SoCs will require different values in cfg0/1, so the two values are stored
> > > separately in the per-SoC data structures.
> > >
> > > For reference, the values are all documented in NV bug 1771116 comment 20.
> > > Rhe ASIC team has validated all these values, except for the Tegra20 value
> > > which is simply left unchanged in this patch.
>
> > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
>
> > > @@ -2078,6 +2070,7 @@ static const struct tegra_pcie_soc_data tegra20_pcie_data = {
> > > .msi_base_shift = 0,
> > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
> > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
> > > + .pads_refclk_cfg0 = 0xfa5cfa5c,
> > > .has_pex_clkreq_en = false,
> > > .has_pex_bias_ctrl = false,
> > > .has_intr_prsnt_sense = false,
> > > @@ -2090,6 +2083,8 @@ static const struct tegra_pcie_soc_data tegra30_pcie_data = {
> > > .msi_base_shift = 8,
> > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> > > + .pads_refclk_cfg0 = 0xfa5cfa5c,
> > > + .pads_refclk_cfg1 = 0xfa5cfa5c,
> > > .has_pex_clkreq_en = true,
> > > .has_pex_bias_ctrl = true,
> > > .has_intr_prsnt_sense = true,
> > > @@ -2102,6 +2097,7 @@ static const struct tegra_pcie_soc_data tegra124_pcie_data = {
> > > .msi_base_shift = 8,
> > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> > > + .pads_refclk_cfg0 = 0x44ac44ac,
> > > .has_pex_clkreq_en = true,
> > > .has_pex_bias_ctrl = true,
> > > .has_intr_prsnt_sense = true,
> >
> > I think it'd be nice to have these decoded into their individual fields,
> > to reduce the magic. We already define the register fields, so it seems
> > sensible to use them.
>
> I did consider that. However, the specification from the ASIC team is always
> the raw values. Decoding them into bitfields is only going to make it harder
> to verify whether the correct values are present (since the reader has to
> manually expand the math), and introduce the possibility of errors during
> the expansion process. I think using raw numbers is better in this case.
Alright, fine with me:
Acked-by: Thierry Reding <treding@nvidia.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] pci: tegra: correctly program PADS_REFCLK registers
2016-06-23 8:44 ` Thierry Reding
@ 2016-06-30 13:20 ` Thierry Reding
2016-06-30 15:35 ` Alex Williamson
0 siblings, 1 reply; 7+ messages in thread
From: Thierry Reding @ 2016-06-30 13:20 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Stephen Warren, linux-tegra, linux-pci, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 3135 bytes --]
On Thu, Jun 23, 2016 at 10:44:48AM +0200, Thierry Reding wrote:
> On Wed, Jun 22, 2016 at 09:34:21AM -0600, Stephen Warren wrote:
> > On 06/22/2016 06:57 AM, Thierry Reding wrote:
> > > On Tue, Jun 21, 2016 at 12:46:40PM -0600, Stephen Warren wrote:
> > > > From: Stephen Warren <swarren@nvidia.com>
> > > >
> > > > The value that should be programmed into the PADS_REFCLK register varies
> > > > per SoC. Fix the Tegra PCIe driver to program the correct values. Future
> > > > SoCs will require different values in cfg0/1, so the two values are stored
> > > > separately in the per-SoC data structures.
> > > >
> > > > For reference, the values are all documented in NV bug 1771116 comment 20.
> > > > Rhe ASIC team has validated all these values, except for the Tegra20 value
> > > > which is simply left unchanged in this patch.
> >
> > > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> >
> > > > @@ -2078,6 +2070,7 @@ static const struct tegra_pcie_soc_data tegra20_pcie_data = {
> > > > .msi_base_shift = 0,
> > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
> > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
> > > > + .pads_refclk_cfg0 = 0xfa5cfa5c,
> > > > .has_pex_clkreq_en = false,
> > > > .has_pex_bias_ctrl = false,
> > > > .has_intr_prsnt_sense = false,
> > > > @@ -2090,6 +2083,8 @@ static const struct tegra_pcie_soc_data tegra30_pcie_data = {
> > > > .msi_base_shift = 8,
> > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> > > > + .pads_refclk_cfg0 = 0xfa5cfa5c,
> > > > + .pads_refclk_cfg1 = 0xfa5cfa5c,
> > > > .has_pex_clkreq_en = true,
> > > > .has_pex_bias_ctrl = true,
> > > > .has_intr_prsnt_sense = true,
> > > > @@ -2102,6 +2097,7 @@ static const struct tegra_pcie_soc_data tegra124_pcie_data = {
> > > > .msi_base_shift = 8,
> > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> > > > + .pads_refclk_cfg0 = 0x44ac44ac,
> > > > .has_pex_clkreq_en = true,
> > > > .has_pex_bias_ctrl = true,
> > > > .has_intr_prsnt_sense = true,
> > >
> > > I think it'd be nice to have these decoded into their individual fields,
> > > to reduce the magic. We already define the register fields, so it seems
> > > sensible to use them.
> >
> > I did consider that. However, the specification from the ASIC team is always
> > the raw values. Decoding them into bitfields is only going to make it harder
> > to verify whether the correct values are present (since the reader has to
> > manually expand the math), and introduce the possibility of errors during
> > the expansion process. I think using raw numbers is better in this case.
>
> Alright, fine with me:
>
> Acked-by: Thierry Reding <treding@nvidia.com>
Hi Bjorn,
I do have a couple of other patches, mostly minor cleanup and prep-work
for 64-bit ARM support, that I'd like to get into v4.8. Would you mind
if I took Stephen's patches into a branch and send it all out via pull
request after it passed testing?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] pci: tegra: correctly program PADS_REFCLK registers
2016-06-30 13:20 ` Thierry Reding
@ 2016-06-30 15:35 ` Alex Williamson
2016-06-30 15:46 ` Thierry Reding
0 siblings, 1 reply; 7+ messages in thread
From: Alex Williamson @ 2016-06-30 15:35 UTC (permalink / raw)
To: Thierry Reding
Cc: Bjorn Helgaas, Stephen Warren, linux-tegra, linux-pci,
Stephen Warren
On Thu, 30 Jun 2016 15:20:01 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:
> On Thu, Jun 23, 2016 at 10:44:48AM +0200, Thierry Reding wrote:
> > On Wed, Jun 22, 2016 at 09:34:21AM -0600, Stephen Warren wrote:
> > > On 06/22/2016 06:57 AM, Thierry Reding wrote:
> > > > On Tue, Jun 21, 2016 at 12:46:40PM -0600, Stephen Warren wrote:
> > > > > From: Stephen Warren <swarren@nvidia.com>
> > > > >
> > > > > The value that should be programmed into the PADS_REFCLK register varies
> > > > > per SoC. Fix the Tegra PCIe driver to program the correct values. Future
> > > > > SoCs will require different values in cfg0/1, so the two values are stored
> > > > > separately in the per-SoC data structures.
> > > > >
> > > > > For reference, the values are all documented in NV bug 1771116 comment 20.
> > > > > Rhe ASIC team has validated all these values, except for the Tegra20 value
> > > > > which is simply left unchanged in this patch.
> > >
> > > > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > >
> > > > > @@ -2078,6 +2070,7 @@ static const struct tegra_pcie_soc_data tegra20_pcie_data = {
> > > > > .msi_base_shift = 0,
> > > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
> > > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
> > > > > + .pads_refclk_cfg0 = 0xfa5cfa5c,
> > > > > .has_pex_clkreq_en = false,
> > > > > .has_pex_bias_ctrl = false,
> > > > > .has_intr_prsnt_sense = false,
> > > > > @@ -2090,6 +2083,8 @@ static const struct tegra_pcie_soc_data tegra30_pcie_data = {
> > > > > .msi_base_shift = 8,
> > > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> > > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> > > > > + .pads_refclk_cfg0 = 0xfa5cfa5c,
> > > > > + .pads_refclk_cfg1 = 0xfa5cfa5c,
> > > > > .has_pex_clkreq_en = true,
> > > > > .has_pex_bias_ctrl = true,
> > > > > .has_intr_prsnt_sense = true,
> > > > > @@ -2102,6 +2097,7 @@ static const struct tegra_pcie_soc_data tegra124_pcie_data = {
> > > > > .msi_base_shift = 8,
> > > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> > > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> > > > > + .pads_refclk_cfg0 = 0x44ac44ac,
> > > > > .has_pex_clkreq_en = true,
> > > > > .has_pex_bias_ctrl = true,
> > > > > .has_intr_prsnt_sense = true,
> > > >
> > > > I think it'd be nice to have these decoded into their individual fields,
> > > > to reduce the magic. We already define the register fields, so it seems
> > > > sensible to use them.
> > >
> > > I did consider that. However, the specification from the ASIC team is always
> > > the raw values. Decoding them into bitfields is only going to make it harder
> > > to verify whether the correct values are present (since the reader has to
> > > manually expand the math), and introduce the possibility of errors during
> > > the expansion process. I think using raw numbers is better in this case.
> >
> > Alright, fine with me:
> >
> > Acked-by: Thierry Reding <treding@nvidia.com>
>
> Hi Bjorn,
>
> I do have a couple of other patches, mostly minor cleanup and prep-work
> for 64-bit ARM support, that I'd like to get into v4.8. Would you mind
> if I took Stephen's patches into a branch and send it all out via pull
> request after it passed testing?
Hi Theirry,
Bjorn is on holiday and will be back at the beginning of the v4.8 merge
window, assuming everything sticks to a regular schedule. I expect
that anything that consolidates and prioritizes potential v4.8 content
for easy evaluation on his return would be appreciated. Thanks,
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] pci: tegra: correctly program PADS_REFCLK registers
2016-06-30 15:35 ` Alex Williamson
@ 2016-06-30 15:46 ` Thierry Reding
0 siblings, 0 replies; 7+ messages in thread
From: Thierry Reding @ 2016-06-30 15:46 UTC (permalink / raw)
To: Alex Williamson
Cc: Bjorn Helgaas, Stephen Warren, linux-tegra, linux-pci,
Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 3917 bytes --]
On Thu, Jun 30, 2016 at 09:35:23AM -0600, Alex Williamson wrote:
> On Thu, 30 Jun 2016 15:20:01 +0200
> Thierry Reding <thierry.reding@gmail.com> wrote:
>
> > On Thu, Jun 23, 2016 at 10:44:48AM +0200, Thierry Reding wrote:
> > > On Wed, Jun 22, 2016 at 09:34:21AM -0600, Stephen Warren wrote:
> > > > On 06/22/2016 06:57 AM, Thierry Reding wrote:
> > > > > On Tue, Jun 21, 2016 at 12:46:40PM -0600, Stephen Warren wrote:
> > > > > > From: Stephen Warren <swarren@nvidia.com>
> > > > > >
> > > > > > The value that should be programmed into the PADS_REFCLK register varies
> > > > > > per SoC. Fix the Tegra PCIe driver to program the correct values. Future
> > > > > > SoCs will require different values in cfg0/1, so the two values are stored
> > > > > > separately in the per-SoC data structures.
> > > > > >
> > > > > > For reference, the values are all documented in NV bug 1771116 comment 20.
> > > > > > Rhe ASIC team has validated all these values, except for the Tegra20 value
> > > > > > which is simply left unchanged in this patch.
> > > >
> > > > > > diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> > > >
> > > > > > @@ -2078,6 +2070,7 @@ static const struct tegra_pcie_soc_data tegra20_pcie_data = {
> > > > > > .msi_base_shift = 0,
> > > > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
> > > > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
> > > > > > + .pads_refclk_cfg0 = 0xfa5cfa5c,
> > > > > > .has_pex_clkreq_en = false,
> > > > > > .has_pex_bias_ctrl = false,
> > > > > > .has_intr_prsnt_sense = false,
> > > > > > @@ -2090,6 +2083,8 @@ static const struct tegra_pcie_soc_data tegra30_pcie_data = {
> > > > > > .msi_base_shift = 8,
> > > > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> > > > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> > > > > > + .pads_refclk_cfg0 = 0xfa5cfa5c,
> > > > > > + .pads_refclk_cfg1 = 0xfa5cfa5c,
> > > > > > .has_pex_clkreq_en = true,
> > > > > > .has_pex_bias_ctrl = true,
> > > > > > .has_intr_prsnt_sense = true,
> > > > > > @@ -2102,6 +2097,7 @@ static const struct tegra_pcie_soc_data tegra124_pcie_data = {
> > > > > > .msi_base_shift = 8,
> > > > > > .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> > > > > > .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> > > > > > + .pads_refclk_cfg0 = 0x44ac44ac,
> > > > > > .has_pex_clkreq_en = true,
> > > > > > .has_pex_bias_ctrl = true,
> > > > > > .has_intr_prsnt_sense = true,
> > > > >
> > > > > I think it'd be nice to have these decoded into their individual fields,
> > > > > to reduce the magic. We already define the register fields, so it seems
> > > > > sensible to use them.
> > > >
> > > > I did consider that. However, the specification from the ASIC team is always
> > > > the raw values. Decoding them into bitfields is only going to make it harder
> > > > to verify whether the correct values are present (since the reader has to
> > > > manually expand the math), and introduce the possibility of errors during
> > > > the expansion process. I think using raw numbers is better in this case.
> > >
> > > Alright, fine with me:
> > >
> > > Acked-by: Thierry Reding <treding@nvidia.com>
> >
> > Hi Bjorn,
> >
> > I do have a couple of other patches, mostly minor cleanup and prep-work
> > for 64-bit ARM support, that I'd like to get into v4.8. Would you mind
> > if I took Stephen's patches into a branch and send it all out via pull
> > request after it passed testing?
>
> Hi Theirry,
>
> Bjorn is on holiday and will be back at the beginning of the v4.8 merge
> window, assuming everything sticks to a regular schedule. I expect
> that anything that consolidates and prioritizes potential v4.8 content
> for easy evaluation on his return would be appreciated. Thanks,
Okay, thanks for letting me know.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-06-30 15:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-21 18:46 [PATCH] pci: tegra: correctly program PADS_REFCLK registers Stephen Warren
2016-06-22 12:57 ` Thierry Reding
2016-06-22 15:34 ` Stephen Warren
2016-06-23 8:44 ` Thierry Reding
2016-06-30 13:20 ` Thierry Reding
2016-06-30 15:35 ` Alex Williamson
2016-06-30 15:46 ` Thierry Reding
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).