Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-04-03 Thread Niklas Cassel 
On Tue, Apr 03, 2018 at 01:53:12PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Apr 02, 2018 at 09:37:03PM +0200, Niklas Cassel wrote:
> > On Thu, Mar 29, 2018 at 03:17:11PM +0530, Kishon Vijay Abraham I wrote:
> > > Hi,
> > > 
> > > On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
> > > > Since a 64-bit BAR consists of a BAR pair, we need to write to both
> > > > BARs in the BAR pair to setup the BAR properly.
> > > > 
> > > > Signed-off-by: Niklas Cassel 
> > > > ---
> > > >  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
> > > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> > > > b/drivers/pci/dwc/pcie-designware-ep.c
> > > > index 5a0bb53c795c..571b90f88d84 100644
> > > > --- a/drivers/pci/dwc/pcie-designware-ep.c
> > > > +++ b/drivers/pci/dwc/pcie-designware-ep.c
> > > > @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, 
> > > > u8 func_no,
> > > > return ret;
> > > >  
> > > > dw_pcie_dbi_ro_wr_en(pci);
> > > > -   dw_pcie_writel_dbi2(pci, reg, size - 1);
> > > > -   dw_pcie_writel_dbi(pci, reg, flags);
> > > > +   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > > > +   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > > > +   dw_pcie_writel_dbi(pci, reg, flags);
> > > > +   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 
> > > > 1));
> > > > +   dw_pcie_writel_dbi(pci, reg + 4, 0);
> > > > +   } else {
> > > > +   dw_pcie_writel_dbi2(pci, reg, size - 1);
> > > > +   dw_pcie_writel_dbi(pci, reg, flags);
> > > > +   }
> > > 
> > > 
> > > I think this should work too?
> > >   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > >   dw_pcie_writel_dbi(pci, reg, flags);
> > > 
> > >   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > >   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> > >   dw_pcie_writel_dbi(pci, reg + 4, 0);
> > >   }
> > > 
> > 
> > Hello Kishon,
> > 
> > I agree, your suggestion is more neat.
> > 
> > 
> > Kishon, please tell me if you insist that the long if-statement
> > in pci_epc_set_bar() should be split, since there are 5 different
> > conditions. Because imho, having 5 succeeding if-statements isn't
> > clearer than having 1 long if-statement.
> > 
> > If Kishon agrees with me, then the review comment in this mail
> > seems to be the only review comment.
> > And in that case, perhaps Lorenzo wouldn't mind fixing this up.
> > Or perhaps Lorenzo prefers if I reroll the whole patch series?
> 
> I updated it myself in my pci/endpoint branch, please have a look, I
> can't guarantee we can merge this for this cycle though, I will ask
> Bjorn; apologies I could not be online for a while.

Hello Lorenzo,

your pci/endpoint branch looks good.

There is no rush, merge it whenever you think is best.

Have in mind that there is EP support @ patchwork for Rockchip
and for pcie-designware-plat, so make sure to juggle all the
branches with care :)

Kind regards,
Niklas

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-04-03 Thread Niklas Cassel 
On Tue, Apr 03, 2018 at 01:53:12PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Apr 02, 2018 at 09:37:03PM +0200, Niklas Cassel wrote:
> > On Thu, Mar 29, 2018 at 03:17:11PM +0530, Kishon Vijay Abraham I wrote:
> > > Hi,
> > > 
> > > On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
> > > > Since a 64-bit BAR consists of a BAR pair, we need to write to both
> > > > BARs in the BAR pair to setup the BAR properly.
> > > > 
> > > > Signed-off-by: Niklas Cassel 
> > > > ---
> > > >  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
> > > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> > > > b/drivers/pci/dwc/pcie-designware-ep.c
> > > > index 5a0bb53c795c..571b90f88d84 100644
> > > > --- a/drivers/pci/dwc/pcie-designware-ep.c
> > > > +++ b/drivers/pci/dwc/pcie-designware-ep.c
> > > > @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, 
> > > > u8 func_no,
> > > > return ret;
> > > >  
> > > > dw_pcie_dbi_ro_wr_en(pci);
> > > > -   dw_pcie_writel_dbi2(pci, reg, size - 1);
> > > > -   dw_pcie_writel_dbi(pci, reg, flags);
> > > > +   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > > > +   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > > > +   dw_pcie_writel_dbi(pci, reg, flags);
> > > > +   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 
> > > > 1));
> > > > +   dw_pcie_writel_dbi(pci, reg + 4, 0);
> > > > +   } else {
> > > > +   dw_pcie_writel_dbi2(pci, reg, size - 1);
> > > > +   dw_pcie_writel_dbi(pci, reg, flags);
> > > > +   }
> > > 
> > > 
> > > I think this should work too?
> > >   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > >   dw_pcie_writel_dbi(pci, reg, flags);
> > > 
> > >   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > >   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> > >   dw_pcie_writel_dbi(pci, reg + 4, 0);
> > >   }
> > > 
> > 
> > Hello Kishon,
> > 
> > I agree, your suggestion is more neat.
> > 
> > 
> > Kishon, please tell me if you insist that the long if-statement
> > in pci_epc_set_bar() should be split, since there are 5 different
> > conditions. Because imho, having 5 succeeding if-statements isn't
> > clearer than having 1 long if-statement.
> > 
> > If Kishon agrees with me, then the review comment in this mail
> > seems to be the only review comment.
> > And in that case, perhaps Lorenzo wouldn't mind fixing this up.
> > Or perhaps Lorenzo prefers if I reroll the whole patch series?
> 
> I updated it myself in my pci/endpoint branch, please have a look, I
> can't guarantee we can merge this for this cycle though, I will ask
> Bjorn; apologies I could not be online for a while.

Hello Lorenzo,

your pci/endpoint branch looks good.

There is no rush, merge it whenever you think is best.

Have in mind that there is EP support @ patchwork for Rockchip
and for pcie-designware-plat, so make sure to juggle all the
branches with care :)

Kind regards,
Niklas

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-04-03 Thread Lorenzo Pieralisi 
On Mon, Apr 02, 2018 at 09:37:03PM +0200, Niklas Cassel wrote:
> On Thu, Mar 29, 2018 at 03:17:11PM +0530, Kishon Vijay Abraham I wrote:
> > Hi,
> > 
> > On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
> > > Since a 64-bit BAR consists of a BAR pair, we need to write to both
> > > BARs in the BAR pair to setup the BAR properly.
> > > 
> > > Signed-off-by: Niklas Cassel 
> > > ---
> > >  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> > > b/drivers/pci/dwc/pcie-designware-ep.c
> > > index 5a0bb53c795c..571b90f88d84 100644
> > > --- a/drivers/pci/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/dwc/pcie-designware-ep.c
> > > @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, 
> > > u8 func_no,
> > >   return ret;
> > >  
> > >   dw_pcie_dbi_ro_wr_en(pci);
> > > - dw_pcie_writel_dbi2(pci, reg, size - 1);
> > > - dw_pcie_writel_dbi(pci, reg, flags);
> > > + if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > > + dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > > + dw_pcie_writel_dbi(pci, reg, flags);
> > > + dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> > > + dw_pcie_writel_dbi(pci, reg + 4, 0);
> > > + } else {
> > > + dw_pcie_writel_dbi2(pci, reg, size - 1);
> > > + dw_pcie_writel_dbi(pci, reg, flags);
> > > + }
> > 
> > 
> > I think this should work too?
> > dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > dw_pcie_writel_dbi(pci, reg, flags);
> > 
> > if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> > dw_pcie_writel_dbi(pci, reg + 4, 0);
> > }
> > 
> 
> Hello Kishon,
> 
> I agree, your suggestion is more neat.
> 
> 
> Kishon, please tell me if you insist that the long if-statement
> in pci_epc_set_bar() should be split, since there are 5 different
> conditions. Because imho, having 5 succeeding if-statements isn't
> clearer than having 1 long if-statement.
> 
> If Kishon agrees with me, then the review comment in this mail
> seems to be the only review comment.
> And in that case, perhaps Lorenzo wouldn't mind fixing this up.
> Or perhaps Lorenzo prefers if I reroll the whole patch series?

I updated it myself in my pci/endpoint branch, please have a look, I
can't guarantee we can merge this for this cycle though, I will ask
Bjorn; apologies I could not be online for a while.

Lorenzo

> Kind regards,
> Niklas

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-04-03 Thread Lorenzo Pieralisi 
On Mon, Apr 02, 2018 at 09:37:03PM +0200, Niklas Cassel wrote:
> On Thu, Mar 29, 2018 at 03:17:11PM +0530, Kishon Vijay Abraham I wrote:
> > Hi,
> > 
> > On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
> > > Since a 64-bit BAR consists of a BAR pair, we need to write to both
> > > BARs in the BAR pair to setup the BAR properly.
> > > 
> > > Signed-off-by: Niklas Cassel 
> > > ---
> > >  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
> > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> > > b/drivers/pci/dwc/pcie-designware-ep.c
> > > index 5a0bb53c795c..571b90f88d84 100644
> > > --- a/drivers/pci/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/dwc/pcie-designware-ep.c
> > > @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, 
> > > u8 func_no,
> > >   return ret;
> > >  
> > >   dw_pcie_dbi_ro_wr_en(pci);
> > > - dw_pcie_writel_dbi2(pci, reg, size - 1);
> > > - dw_pcie_writel_dbi(pci, reg, flags);
> > > + if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > > + dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > > + dw_pcie_writel_dbi(pci, reg, flags);
> > > + dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> > > + dw_pcie_writel_dbi(pci, reg + 4, 0);
> > > + } else {
> > > + dw_pcie_writel_dbi2(pci, reg, size - 1);
> > > + dw_pcie_writel_dbi(pci, reg, flags);
> > > + }
> > 
> > 
> > I think this should work too?
> > dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > dw_pcie_writel_dbi(pci, reg, flags);
> > 
> > if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> > dw_pcie_writel_dbi(pci, reg + 4, 0);
> > }
> > 
> 
> Hello Kishon,
> 
> I agree, your suggestion is more neat.
> 
> 
> Kishon, please tell me if you insist that the long if-statement
> in pci_epc_set_bar() should be split, since there are 5 different
> conditions. Because imho, having 5 succeeding if-statements isn't
> clearer than having 1 long if-statement.
> 
> If Kishon agrees with me, then the review comment in this mail
> seems to be the only review comment.
> And in that case, perhaps Lorenzo wouldn't mind fixing this up.
> Or perhaps Lorenzo prefers if I reroll the whole patch series?

I updated it myself in my pci/endpoint branch, please have a look, I
can't guarantee we can merge this for this cycle though, I will ask
Bjorn; apologies I could not be online for a while.

Lorenzo

> Kind regards,
> Niklas

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-04-02 Thread Kishon Vijay Abraham I 


On Tuesday 03 April 2018 01:07 AM, Niklas Cassel wrote:
> On Thu, Mar 29, 2018 at 03:17:11PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
>>> Since a 64-bit BAR consists of a BAR pair, we need to write to both
>>> BARs in the BAR pair to setup the BAR properly.
>>>
>>> Signed-off-by: Niklas Cassel 
>>> ---
>>>  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
>>> b/drivers/pci/dwc/pcie-designware-ep.c
>>> index 5a0bb53c795c..571b90f88d84 100644
>>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>>> @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
>>> func_no,
>>> return ret;
>>>  
>>> dw_pcie_dbi_ro_wr_en(pci);
>>> -   dw_pcie_writel_dbi2(pci, reg, size - 1);
>>> -   dw_pcie_writel_dbi(pci, reg, flags);
>>> +   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
>>> +   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
>>> +   dw_pcie_writel_dbi(pci, reg, flags);
>>> +   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
>>> +   dw_pcie_writel_dbi(pci, reg + 4, 0);
>>> +   } else {
>>> +   dw_pcie_writel_dbi2(pci, reg, size - 1);
>>> +   dw_pcie_writel_dbi(pci, reg, flags);
>>> +   }
>>
>>
>> I think this should work too?
>>  dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
>>  dw_pcie_writel_dbi(pci, reg, flags);
>>
>>  if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
>>  dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
>>  dw_pcie_writel_dbi(pci, reg + 4, 0);
>>  }
>>
> 
> Hello Kishon,
> 
> I agree, your suggestion is more neat.
> 
> 
> Kishon, please tell me if you insist that the long if-statement
> in pci_epc_set_bar() should be split, since there are 5 different
> conditions. Because imho, having 5 succeeding if-statements isn't

I'm okay as it is as well if Lorenzo/Bjorn is also fine with it.

Thanks
Kishon

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-04-02 Thread Kishon Vijay Abraham I 


On Tuesday 03 April 2018 01:07 AM, Niklas Cassel wrote:
> On Thu, Mar 29, 2018 at 03:17:11PM +0530, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
>>> Since a 64-bit BAR consists of a BAR pair, we need to write to both
>>> BARs in the BAR pair to setup the BAR properly.
>>>
>>> Signed-off-by: Niklas Cassel 
>>> ---
>>>  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
>>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
>>> b/drivers/pci/dwc/pcie-designware-ep.c
>>> index 5a0bb53c795c..571b90f88d84 100644
>>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>>> @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
>>> func_no,
>>> return ret;
>>>  
>>> dw_pcie_dbi_ro_wr_en(pci);
>>> -   dw_pcie_writel_dbi2(pci, reg, size - 1);
>>> -   dw_pcie_writel_dbi(pci, reg, flags);
>>> +   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
>>> +   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
>>> +   dw_pcie_writel_dbi(pci, reg, flags);
>>> +   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
>>> +   dw_pcie_writel_dbi(pci, reg + 4, 0);
>>> +   } else {
>>> +   dw_pcie_writel_dbi2(pci, reg, size - 1);
>>> +   dw_pcie_writel_dbi(pci, reg, flags);
>>> +   }
>>
>>
>> I think this should work too?
>>  dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
>>  dw_pcie_writel_dbi(pci, reg, flags);
>>
>>  if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
>>  dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
>>  dw_pcie_writel_dbi(pci, reg + 4, 0);
>>  }
>>
> 
> Hello Kishon,
> 
> I agree, your suggestion is more neat.
> 
> 
> Kishon, please tell me if you insist that the long if-statement
> in pci_epc_set_bar() should be split, since there are 5 different
> conditions. Because imho, having 5 succeeding if-statements isn't

I'm okay as it is as well if Lorenzo/Bjorn is also fine with it.

Thanks
Kishon

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-04-02 Thread Niklas Cassel 
On Thu, Mar 29, 2018 at 03:17:11PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
> > Since a 64-bit BAR consists of a BAR pair, we need to write to both
> > BARs in the BAR pair to setup the BAR properly.
> > 
> > Signed-off-by: Niklas Cassel 
> > ---
> >  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> > b/drivers/pci/dwc/pcie-designware-ep.c
> > index 5a0bb53c795c..571b90f88d84 100644
> > --- a/drivers/pci/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/dwc/pcie-designware-ep.c
> > @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
> > func_no,
> > return ret;
> >  
> > dw_pcie_dbi_ro_wr_en(pci);
> > -   dw_pcie_writel_dbi2(pci, reg, size - 1);
> > -   dw_pcie_writel_dbi(pci, reg, flags);
> > +   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > +   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > +   dw_pcie_writel_dbi(pci, reg, flags);
> > +   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> > +   dw_pcie_writel_dbi(pci, reg + 4, 0);
> > +   } else {
> > +   dw_pcie_writel_dbi2(pci, reg, size - 1);
> > +   dw_pcie_writel_dbi(pci, reg, flags);
> > +   }
> 
> 
> I think this should work too?
>   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
>   dw_pcie_writel_dbi(pci, reg, flags);
> 
>   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
>   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
>   dw_pcie_writel_dbi(pci, reg + 4, 0);
>   }
> 

Hello Kishon,

I agree, your suggestion is more neat.


Kishon, please tell me if you insist that the long if-statement
in pci_epc_set_bar() should be split, since there are 5 different
conditions. Because imho, having 5 succeeding if-statements isn't
clearer than having 1 long if-statement.

If Kishon agrees with me, then the review comment in this mail
seems to be the only review comment.
And in that case, perhaps Lorenzo wouldn't mind fixing this up.
Or perhaps Lorenzo prefers if I reroll the whole patch series?

Kind regards,
Niklas

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-04-02 Thread Niklas Cassel 
On Thu, Mar 29, 2018 at 03:17:11PM +0530, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
> > Since a 64-bit BAR consists of a BAR pair, we need to write to both
> > BARs in the BAR pair to setup the BAR properly.
> > 
> > Signed-off-by: Niklas Cassel 
> > ---
> >  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> > b/drivers/pci/dwc/pcie-designware-ep.c
> > index 5a0bb53c795c..571b90f88d84 100644
> > --- a/drivers/pci/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/dwc/pcie-designware-ep.c
> > @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
> > func_no,
> > return ret;
> >  
> > dw_pcie_dbi_ro_wr_en(pci);
> > -   dw_pcie_writel_dbi2(pci, reg, size - 1);
> > -   dw_pcie_writel_dbi(pci, reg, flags);
> > +   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > +   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> > +   dw_pcie_writel_dbi(pci, reg, flags);
> > +   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> > +   dw_pcie_writel_dbi(pci, reg + 4, 0);
> > +   } else {
> > +   dw_pcie_writel_dbi2(pci, reg, size - 1);
> > +   dw_pcie_writel_dbi(pci, reg, flags);
> > +   }
> 
> 
> I think this should work too?
>   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
>   dw_pcie_writel_dbi(pci, reg, flags);
> 
>   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
>   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
>   dw_pcie_writel_dbi(pci, reg + 4, 0);
>   }
> 

Hello Kishon,

I agree, your suggestion is more neat.


Kishon, please tell me if you insist that the long if-statement
in pci_epc_set_bar() should be split, since there are 5 different
conditions. Because imho, having 5 succeeding if-statements isn't
clearer than having 1 long if-statement.

If Kishon agrees with me, then the review comment in this mail
seems to be the only review comment.
And in that case, perhaps Lorenzo wouldn't mind fixing this up.
Or perhaps Lorenzo prefers if I reroll the whole patch series?

Kind regards,
Niklas

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-03-29 Thread Kishon Vijay Abraham I 
Hi,

On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
> Since a 64-bit BAR consists of a BAR pair, we need to write to both
> BARs in the BAR pair to setup the BAR properly.
> 
> Signed-off-by: Niklas Cassel 
> ---
>  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> b/drivers/pci/dwc/pcie-designware-ep.c
> index 5a0bb53c795c..571b90f88d84 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
> func_no,
>   return ret;
>  
>   dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writel_dbi2(pci, reg, size - 1);
> - dw_pcie_writel_dbi(pci, reg, flags);
> + if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> + dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg, flags);
> + dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg + 4, 0);
> + } else {
> + dw_pcie_writel_dbi2(pci, reg, size - 1);
> + dw_pcie_writel_dbi(pci, reg, flags);
> + }


I think this should work too?
dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
dw_pcie_writel_dbi(pci, reg, flags);

if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
dw_pcie_writel_dbi(pci, reg + 4, 0);
}

Thanks
Kishon

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-03-29 Thread Kishon Vijay Abraham I 
Hi,

On Wednesday 28 March 2018 05:20 PM, Niklas Cassel wrote:
> Since a 64-bit BAR consists of a BAR pair, we need to write to both
> BARs in the BAR pair to setup the BAR properly.
> 
> Signed-off-by: Niklas Cassel 
> ---
>  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> b/drivers/pci/dwc/pcie-designware-ep.c
> index 5a0bb53c795c..571b90f88d84 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
> func_no,
>   return ret;
>  
>   dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writel_dbi2(pci, reg, size - 1);
> - dw_pcie_writel_dbi(pci, reg, flags);
> + if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> + dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg, flags);
> + dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg + 4, 0);
> + } else {
> + dw_pcie_writel_dbi2(pci, reg, size - 1);
> + dw_pcie_writel_dbi(pci, reg, flags);
> + }


I think this should work too?
dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
dw_pcie_writel_dbi(pci, reg, flags);

if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
dw_pcie_writel_dbi(pci, reg + 4, 0);
}

Thanks
Kishon

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-03-28 Thread Gustavo Pimentel 
Hi Niklas,

On 28/03/2018 12:50, Niklas Cassel wrote:
> Since a 64-bit BAR consists of a BAR pair, we need to write to both
> BARs in the BAR pair to setup the BAR properly.
> 
> Signed-off-by: Niklas Cassel 
> ---
>  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> b/drivers/pci/dwc/pcie-designware-ep.c
> index 5a0bb53c795c..571b90f88d84 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
> func_no,
>   return ret;
>  
>   dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writel_dbi2(pci, reg, size - 1);
> - dw_pcie_writel_dbi(pci, reg, flags);
> + if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> + dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg, flags);
> + dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg + 4, 0);
> + } else {
> + dw_pcie_writel_dbi2(pci, reg, size - 1);
> + dw_pcie_writel_dbi(pci, reg, flags);
> + }
>   dw_pcie_dbi_ro_wr_dis(pci);
>  
>   return 0;
> 

Seems good to me. :)

Reviewed-by: Gustavo Pimentel

Re: [PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-03-28 Thread Gustavo Pimentel 
Hi Niklas,

On 28/03/2018 12:50, Niklas Cassel wrote:
> Since a 64-bit BAR consists of a BAR pair, we need to write to both
> BARs in the BAR pair to setup the BAR properly.
> 
> Signed-off-by: Niklas Cassel 
> ---
>  drivers/pci/dwc/pcie-designware-ep.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> b/drivers/pci/dwc/pcie-designware-ep.c
> index 5a0bb53c795c..571b90f88d84 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
> func_no,
>   return ret;
>  
>   dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writel_dbi2(pci, reg, size - 1);
> - dw_pcie_writel_dbi(pci, reg, flags);
> + if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> + dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg, flags);
> + dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
> + dw_pcie_writel_dbi(pci, reg + 4, 0);
> + } else {
> + dw_pcie_writel_dbi2(pci, reg, size - 1);
> + dw_pcie_writel_dbi(pci, reg, flags);
> + }
>   dw_pcie_dbi_ro_wr_dis(pci);
>  
>   return 0;
> 

Seems good to me. :)

Reviewed-by: Gustavo Pimentel

[PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-03-28 Thread Niklas Cassel 
Since a 64-bit BAR consists of a BAR pair, we need to write to both
BARs in the BAR pair to setup the BAR properly.

Signed-off-by: Niklas Cassel 
---
 drivers/pci/dwc/pcie-designware-ep.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
b/drivers/pci/dwc/pcie-designware-ep.c
index 5a0bb53c795c..571b90f88d84 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
func_no,
return ret;
 
dw_pcie_dbi_ro_wr_en(pci);
-   dw_pcie_writel_dbi2(pci, reg, size - 1);
-   dw_pcie_writel_dbi(pci, reg, flags);
+   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
+   dw_pcie_writel_dbi(pci, reg, flags);
+   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
+   dw_pcie_writel_dbi(pci, reg + 4, 0);
+   } else {
+   dw_pcie_writel_dbi2(pci, reg, size - 1);
+   dw_pcie_writel_dbi(pci, reg, flags);
+   }
dw_pcie_dbi_ro_wr_dis(pci);
 
return 0;
-- 
2.14.2

[PATCH v5 06/12] PCI: designware-ep: Make dw_pcie_ep_set_bar() handle 64-bit BARs properly

2018-03-28 Thread Niklas Cassel 
Since a 64-bit BAR consists of a BAR pair, we need to write to both
BARs in the BAR pair to setup the BAR properly.

Signed-off-by: Niklas Cassel 
---
 drivers/pci/dwc/pcie-designware-ep.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
b/drivers/pci/dwc/pcie-designware-ep.c
index 5a0bb53c795c..571b90f88d84 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -138,8 +138,15 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 
func_no,
return ret;
 
dw_pcie_dbi_ro_wr_en(pci);
-   dw_pcie_writel_dbi2(pci, reg, size - 1);
-   dw_pcie_writel_dbi(pci, reg, flags);
+   if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
+   dw_pcie_writel_dbi2(pci, reg, lower_32_bits(size - 1));
+   dw_pcie_writel_dbi(pci, reg, flags);
+   dw_pcie_writel_dbi2(pci, reg + 4, upper_32_bits(size - 1));
+   dw_pcie_writel_dbi(pci, reg + 4, 0);
+   } else {
+   dw_pcie_writel_dbi2(pci, reg, size - 1);
+   dw_pcie_writel_dbi(pci, reg, flags);
+   }
dw_pcie_dbi_ro_wr_dis(pci);
 
return 0;
-- 
2.14.2