2017-07-14 12:07:52

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH 0/2] PCI: dwc: convert remaining dbi read/writes to dw_pcie_readX_dbi/dw_pcie_writeX_dbi

Since the introduction of the dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros,
most dw_pcie_read(pci->dbi_base, ..)/dw_pcie_write(pci->dbi_base, ..) calls
have been converted to dw_pcie_readX_dbi/dw_pcie_writeX_dbi calls.
Convert the remaining calls.

Niklas Cassel (2):
PCI: dwc: dra7xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros
PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi
macros

drivers/pci/dwc/pci-dra7xx.c | 14 ++++++--------
drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
2 files changed, 16 insertions(+), 20 deletions(-)

--
2.11.0


2017-07-14 12:07:56

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH 2/2] PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros

Signed-off-by: Niklas Cassel <[email protected]>
---
drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/dwc/pcie-spear13xx.c b/drivers/pci/dwc/pcie-spear13xx.c
index 80897291e0fb..7ebfbf6086fe 100644
--- a/drivers/pci/dwc/pcie-spear13xx.c
+++ b/drivers/pci/dwc/pcie-spear13xx.c
@@ -92,34 +92,32 @@ static int spear13xx_pcie_establish_link(struct spear13xx_pcie *spear13xx_pcie)
* default value in capability register is 512 bytes. So force
* it to 128 here.
*/
- dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, &val);
+ val = dw_pcie_readw_dbi(pci, exp_cap_off + PCI_EXP_DEVCTL);
val &= ~PCI_EXP_DEVCTL_READRQ;
- dw_pcie_write(pci->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, val);
+ dw_pcie_writew_dbi(pci, exp_cap_off + PCI_EXP_DEVCTL, val);

- dw_pcie_write(pci->dbi_base + PCI_VENDOR_ID, 2, 0x104A);
- dw_pcie_write(pci->dbi_base + PCI_DEVICE_ID, 2, 0xCD80);
+ dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, 0x104A);
+ dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, 0xCD80);

/*
* if is_gen1 is set then handle it, so that some buggy card
* also works
*/
if (spear13xx_pcie->is_gen1) {
- dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
- 4, &val);
+ val = dw_pcie_readl_dbi(pci, exp_cap_off + PCI_EXP_LNKCAP);
if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
val &= ~((u32)PCI_EXP_LNKCAP_SLS);
val |= PCI_EXP_LNKCAP_SLS_2_5GB;
- dw_pcie_write(pci->dbi_base + exp_cap_off +
- PCI_EXP_LNKCAP, 4, val);
+ dw_pcie_writel_dbi(pci, exp_cap_off + PCI_EXP_LNKCAP,
+ val);
}

- dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
- 2, &val);
+ val = dw_pcie_readw_dbi(pci, exp_cap_off + PCI_EXP_LNKCTL2);
if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
val &= ~((u32)PCI_EXP_LNKCAP_SLS);
val |= PCI_EXP_LNKCAP_SLS_2_5GB;
- dw_pcie_write(pci->dbi_base + exp_cap_off +
- PCI_EXP_LNKCTL2, 2, val);
+ dw_pcie_writew_dbi(pci, exp_cap_off + PCI_EXP_LNKCTL2,
+ val);
}
}

--
2.11.0

2017-07-14 12:07:54

by Niklas Cassel

[permalink] [raw]
Subject: [PATCH 1/2] PCI: dwc: dra7xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros

Signed-off-by: Niklas Cassel <[email protected]>
---
drivers/pci/dwc/pci-dra7xx.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
index f2fc5f47064e..e30c837c4437 100644
--- a/drivers/pci/dwc/pci-dra7xx.c
+++ b/drivers/pci/dwc/pci-dra7xx.c
@@ -145,22 +145,20 @@ static int dra7xx_pcie_establish_link(struct dw_pcie *pci)
}

if (dra7xx->link_gen == 1) {
- dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
- 4, &reg);
+ reg = dw_pcie_readl_dbi(pci, exp_cap_off + PCI_EXP_LNKCAP);
if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
- dw_pcie_write(pci->dbi_base + exp_cap_off +
- PCI_EXP_LNKCAP, 4, reg);
+ dw_pcie_writel_dbi(pci, exp_cap_off + PCI_EXP_LNKCAP,
+ reg);
}

- dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
- 2, &reg);
+ reg = dw_pcie_readw_dbi(pci, exp_cap_off + PCI_EXP_LNKCTL2);
if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
- dw_pcie_write(pci->dbi_base + exp_cap_off +
- PCI_EXP_LNKCTL2, 2, reg);
+ dw_pcie_writew_dbi(pci, exp_cap_off + PCI_EXP_LNKCTL2,
+ reg);
}
}

--
2.11.0

2017-08-02 21:33:00

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: dwc: convert remaining dbi read/writes to dw_pcie_readX_dbi/dw_pcie_writeX_dbi

[+cc Kishon, Pratyush]

On Fri, Jul 14, 2017 at 02:07:33PM +0200, Niklas Cassel wrote:
> Since the introduction of the dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros,
> most dw_pcie_read(pci->dbi_base, ..)/dw_pcie_write(pci->dbi_base, ..) calls
> have been converted to dw_pcie_readX_dbi/dw_pcie_writeX_dbi calls.
> Convert the remaining calls.
>
> Niklas Cassel (2):
> PCI: dwc: dra7xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros
> PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi
> macros
>
> drivers/pci/dwc/pci-dra7xx.c | 14 ++++++--------
> drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
> 2 files changed, 16 insertions(+), 20 deletions(-)

Waiting for ack from maintainers (cc'd).

2017-08-03 05:03:37

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: dwc: convert remaining dbi read/writes to dw_pcie_readX_dbi/dw_pcie_writeX_dbi

+cc kishon for dra7xx

On Friday 14 July 2017 05:37 PM, Niklas Cassel wrote:
> Since the introduction of the dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros,
> most dw_pcie_read(pci->dbi_base, ..)/dw_pcie_write(pci->dbi_base, ..) calls
> have been converted to dw_pcie_readX_dbi/dw_pcie_writeX_dbi calls.
> Convert the remaining calls.
>
> Niklas Cassel (2):
> PCI: dwc: dra7xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros
> PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi
> macros
>
> drivers/pci/dwc/pci-dra7xx.c | 14 ++++++--------
> drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
> 2 files changed, 16 insertions(+), 20 deletions(-)
>

--
Regards
Vignesh

2017-08-03 20:12:18

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: dwc: convert remaining dbi read/writes to dw_pcie_readX_dbi/dw_pcie_writeX_dbi

Adding Kishon & Pratyush for real this time.

On Wed, Aug 02, 2017 at 04:32:56PM -0500, Bjorn Helgaas wrote:
> [+cc Kishon, Pratyush]
>
> On Fri, Jul 14, 2017 at 02:07:33PM +0200, Niklas Cassel wrote:
> > Since the introduction of the dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros,
> > most dw_pcie_read(pci->dbi_base, ..)/dw_pcie_write(pci->dbi_base, ..) calls
> > have been converted to dw_pcie_readX_dbi/dw_pcie_writeX_dbi calls.
> > Convert the remaining calls.
> >
> > Niklas Cassel (2):
> > PCI: dwc: dra7xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros
> > PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi
> > macros
> >
> > drivers/pci/dwc/pci-dra7xx.c | 14 ++++++--------
> > drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
> > 2 files changed, 16 insertions(+), 20 deletions(-)
>
> Waiting for ack from maintainers (cc'd).

2017-08-04 05:19:32

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: dwc: convert remaining dbi read/writes to dw_pcie_readX_dbi/dw_pcie_writeX_dbi



On Friday 04 August 2017 01:42 AM, Bjorn Helgaas wrote:
> Adding Kishon & Pratyush for real this time.
>
> On Wed, Aug 02, 2017 at 04:32:56PM -0500, Bjorn Helgaas wrote:
>> [+cc Kishon, Pratyush]
>>
>> On Fri, Jul 14, 2017 at 02:07:33PM +0200, Niklas Cassel wrote:
>>> Since the introduction of the dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros,
>>> most dw_pcie_read(pci->dbi_base, ..)/dw_pcie_write(pci->dbi_base, ..) calls
>>> have been converted to dw_pcie_readX_dbi/dw_pcie_writeX_dbi calls.
>>> Convert the remaining calls.
>>>
>>> Niklas Cassel (2):
>>> PCI: dwc: dra7xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros
>>> PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi
>>> macros
>>>
>>> drivers/pci/dwc/pci-dra7xx.c | 14 ++++++--------
>>> drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
>>> 2 files changed, 16 insertions(+), 20 deletions(-)
>>
>> Waiting for ack from maintainers (cc'd).

Tested this in dra7xx
Acked-by: Kishon Vijay Abraham I <[email protected]>

2017-08-14 18:09:16

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 2/2] PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros

On Fri, Jul 14, 2017 at 02:07:35PM +0200, Niklas Cassel wrote:
> Signed-off-by: Niklas Cassel <[email protected]>

Pratyush, are you OK with this?

> ---
> drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/dwc/pcie-spear13xx.c b/drivers/pci/dwc/pcie-spear13xx.c
> index 80897291e0fb..7ebfbf6086fe 100644
> --- a/drivers/pci/dwc/pcie-spear13xx.c
> +++ b/drivers/pci/dwc/pcie-spear13xx.c
> @@ -92,34 +92,32 @@ static int spear13xx_pcie_establish_link(struct spear13xx_pcie *spear13xx_pcie)
> * default value in capability register is 512 bytes. So force
> * it to 128 here.
> */
> - dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, &val);
> + val = dw_pcie_readw_dbi(pci, exp_cap_off + PCI_EXP_DEVCTL);
> val &= ~PCI_EXP_DEVCTL_READRQ;
> - dw_pcie_write(pci->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, val);
> + dw_pcie_writew_dbi(pci, exp_cap_off + PCI_EXP_DEVCTL, val);
>
> - dw_pcie_write(pci->dbi_base + PCI_VENDOR_ID, 2, 0x104A);
> - dw_pcie_write(pci->dbi_base + PCI_DEVICE_ID, 2, 0xCD80);
> + dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, 0x104A);
> + dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, 0xCD80);
>
> /*
> * if is_gen1 is set then handle it, so that some buggy card
> * also works
> */
> if (spear13xx_pcie->is_gen1) {
> - dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> - 4, &val);
> + val = dw_pcie_readl_dbi(pci, exp_cap_off + PCI_EXP_LNKCAP);
> if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> val &= ~((u32)PCI_EXP_LNKCAP_SLS);
> val |= PCI_EXP_LNKCAP_SLS_2_5GB;
> - dw_pcie_write(pci->dbi_base + exp_cap_off +
> - PCI_EXP_LNKCAP, 4, val);
> + dw_pcie_writel_dbi(pci, exp_cap_off + PCI_EXP_LNKCAP,
> + val);
> }
>
> - dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> - 2, &val);
> + val = dw_pcie_readw_dbi(pci, exp_cap_off + PCI_EXP_LNKCTL2);
> if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
> val &= ~((u32)PCI_EXP_LNKCAP_SLS);
> val |= PCI_EXP_LNKCAP_SLS_2_5GB;
> - dw_pcie_write(pci->dbi_base + exp_cap_off +
> - PCI_EXP_LNKCTL2, 2, val);
> + dw_pcie_writew_dbi(pci, exp_cap_off + PCI_EXP_LNKCTL2,
> + val);
> }
> }
>
> --
> 2.11.0
>

2017-08-15 22:39:50

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: dwc: convert remaining dbi read/writes to dw_pcie_readX_dbi/dw_pcie_writeX_dbi

[+cc Kishon, Pratyush]

On Fri, Jul 14, 2017 at 02:07:33PM +0200, Niklas Cassel wrote:
> Since the introduction of the dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros,
> most dw_pcie_read(pci->dbi_base, ..)/dw_pcie_write(pci->dbi_base, ..) calls
> have been converted to dw_pcie_readX_dbi/dw_pcie_writeX_dbi calls.
> Convert the remaining calls.
>
> Niklas Cassel (2):
> PCI: dwc: dra7xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros
> PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi
> macros
>
> drivers/pci/dwc/pci-dra7xx.c | 14 ++++++--------
> drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
> 2 files changed, 16 insertions(+), 20 deletions(-)

These patches are short, but obscure, e.g.,

- dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_LNKCAP, 4, &val);
+ val = dw_pcie_readl_dbi(pci, exp_cap_off + PCI_EXP_LNKCAP);

The original call path was:

dw_pcie_read(addr, ...)
readl(addr)

and we're replacing it with the much more complicated:

dw_pcie_readl_dbi
__dw_pcie_read_dbi
if (pci->ops->read_dbi)
return pci->ops->read_dbi(pci, base, reg, size)
dw_pcie_read(base + reg, size, &val)
readl

This is not functionally equivalent (the new path uses ops->read_dbi
if it's set), so the changelog should say something about why we need
this change.

Neither dra7xx nor spear13xx sets .read_dbi, so my guess is this
doesn't actually fix anything that's broken and this is just for
consistency.

That's fine, but I'm confused because I looked for some of the
previous conversions you mentioned but couldn't find any. I'm lost in
a maze of twisty little passages.

Bjorn

2017-09-04 16:11:01

by Niklas Cassel

[permalink] [raw]
Subject: Re: [PATCH 0/2] PCI: dwc: convert remaining dbi read/writes to dw_pcie_readX_dbi/dw_pcie_writeX_dbi

On 08/16/2017 12:39 AM, Bjorn Helgaas wrote:
> [+cc Kishon, Pratyush]
>
> On Fri, Jul 14, 2017 at 02:07:33PM +0200, Niklas Cassel wrote:
>> Since the introduction of the dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros,
>> most dw_pcie_read(pci->dbi_base, ..)/dw_pcie_write(pci->dbi_base, ..) calls
>> have been converted to dw_pcie_readX_dbi/dw_pcie_writeX_dbi calls.
>> Convert the remaining calls.
>>
>> Niklas Cassel (2):
>> PCI: dwc: dra7xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros
>> PCI: dwc: spear13xx: utilize dw_pcie_readX_dbi/dw_pcie_writeX_dbi
>> macros
>>
>> drivers/pci/dwc/pci-dra7xx.c | 14 ++++++--------
>> drivers/pci/dwc/pcie-spear13xx.c | 22 ++++++++++------------
>> 2 files changed, 16 insertions(+), 20 deletions(-)
>
> These patches are short, but obscure, e.g.,
>
> - dw_pcie_read(pci->dbi_base + exp_cap_off + PCI_EXP_LNKCAP, 4, &val);
> + val = dw_pcie_readl_dbi(pci, exp_cap_off + PCI_EXP_LNKCAP);
>
> The original call path was:
>
> dw_pcie_read(addr, ...)
> readl(addr)
>
> and we're replacing it with the much more complicated:
>
> dw_pcie_readl_dbi
> __dw_pcie_read_dbi
> if (pci->ops->read_dbi)
> return pci->ops->read_dbi(pci, base, reg, size)
> dw_pcie_read(base + reg, size, &val)
> readl
>
> This is not functionally equivalent (the new path uses ops->read_dbi
> if it's set), so the changelog should say something about why we need
> this change.
>
> Neither dra7xx nor spear13xx sets .read_dbi, so my guess is this
> doesn't actually fix anything that's broken and this is just for
> consistency.

Hello Bjorn,

It is just for consistency.

Sorry for my misleading cover letter.
It should have said: "For consistency, convert all
dw_pcie_read(pci->dbi_base, ..)/dw_pcie_write(pci->dbi_base, ..)
calls to use the dw_pcie_readX_dbi/dw_pcie_writeX_dbi macros".

It is true that dw_pcie_readX_dbi/dw_pcie_writeX_dbi will use
ops->read_dbi if it is set. However, if ops->read_dbi is set,
you will most likely want to use that function for reading the dbi.
Anyway, neither spear13xx nor dra7xx has ops->read_dbi set,
so these patches shouldn't change their current behavior.

Do you want me to resend these patches or would you prefer
to just drop them?

Best regards,
Niklas

>
> That's fine, but I'm confused because I looked for some of the
> previous conversions you mentioned but couldn't find any. I'm lost in
> a maze of twisty little passages.
>
> Bjorn
>