Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1594173 > unrolled thread

Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support

Started byJoao Pinto <Joao.Pinto@synopsys.com>
First post2017-03-07 13:30 +0100
Last post2017-03-08 17:10 +0100
Articles 9 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-07 13:30 +0100
    Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-08 12:40 +0100
      Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Kishon Vijay Abraham I <kishon@ti.com> - 2017-03-08 12:50 +0100
        Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-08 12:50 +0100
          Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-08 16:40 +0100
            Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-08 16:40 +0100
            Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Christoph Hellwig <hch@infradead.org> - 2017-03-08 20:20 +0100
              Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Joao Pinto <Joao.Pinto@synopsys.com> - 2017-03-09 13:00 +0100
          Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support Kishon Vijay Abraham I <kishon@ti.com> - 2017-03-08 17:10 +0100

#1594173 — Re: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support

FromJoao Pinto <Joao.Pinto@synopsys.com>
Date2017-03-07 13:30 +0100
SubjectRe: [PATCH v2 08/22] PCI: dwc: designware: Add EP mode support
Message-ID<timpA-7rg-25@gated-at.bofh.it>
Hi Kishon,

Às 5:18 AM de 3/7/2017, Kishon Vijay Abraham I escreveu:
> Hi Joao,
> 
> On Friday 17 February 2017 10:50 PM, Joao Pinto wrote:
>> Às 9:50 AM de 2/17/2017, Kishon Vijay Abraham I escreveu:
>>> Add endpoint mode support to designware driver. This uses the
>>> EP Core layer introduced recently to add endpoint mode support.
>>> *Any* function driver can now use this designware device
>>> in order to achieve the EP functionality.
>>>
>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>> ---
>>>  drivers/pci/dwc/Kconfig              |    5 +
>>>  drivers/pci/dwc/Makefile             |    1 +
>>>  drivers/pci/dwc/pcie-designware-ep.c |  342 ++++++++++++++++++++++++++++++++++
>>>  drivers/pci/dwc/pcie-designware.c    |   51 +++++
>>>  drivers/pci/dwc/pcie-designware.h    |   72 +++++++
>>>  5 files changed, 471 insertions(+)
>>>  create mode 100644 drivers/pci/dwc/pcie-designware-ep.c
>>>
> 
> <snip>
> 
>>> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
>>> index 686945d..49b28c8 100644
>>> --- a/drivers/pci/dwc/pcie-designware.c
>>> +++ b/drivers/pci/dwc/pcie-designware.c
>>> @@ -173,6 +173,57 @@ void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>>>  	dev_err(pci->dev, "iATU is not being enabled\n");
>>>  }
>>>  
>>> +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
>>> +			     u64 cpu_addr, enum dw_pcie_as_type as_type)
>>> +{
>>> +	int type;
>>> +	void __iomem *base = pci->dbi_base;
>>> +
>>> +	dw_pcie_write_dbi(pci, base, PCIE_ATU_VIEWPORT, 0x4,
>>> +			  PCIE_ATU_REGION_INBOUND | index);
>>> +	dw_pcie_write_dbi(pci, base, PCIE_ATU_LOWER_TARGET, 0x4,
>>> +			  lower_32_bits(cpu_addr));
>>> +	dw_pcie_write_dbi(pci, base, PCIE_ATU_UPPER_TARGET, 0x4,
>>> +			  upper_32_bits(cpu_addr));
>>> +
>>> +	switch (as_type) {
>>> +	case DW_PCIE_AS_MEM:
>>> +		type = PCIE_ATU_TYPE_MEM;
>>> +		break;
>>> +	case DW_PCIE_AS_IO:
>>> +		type = PCIE_ATU_TYPE_IO;
>>> +		break;
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	dw_pcie_write_dbi(pci, base, PCIE_ATU_CR1, 0x4, type);
>>> +	dw_pcie_write_dbi(pci, base, PCIE_ATU_CR2, 0x4, PCIE_ATU_ENABLE |
>>> +			  PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
>>> +	return 0;
>>> +}
>>> +
>>
>> This Atu programming is for PCI Cores <= 4.70. Please follow the same approach as:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_cgit_linux_kernel_git_helgaas_pci.git_tree_drivers_pci_dwc_pcie-2Ddesignware.c-3Fh-3Dpci_host-2Ddesignware-23n95&d=DwID-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=s2fO0hii0OGNOv9qQy_HRXy-xAJUD1NNoEcc3io_kx0&m=MqqHFJBR0jj9ZQILcUJEd-CQkTihuOSf69e-XxZJvRs&s=fY5N7Mt9iszsAI04DPm-cSC6cSE5P2axHUFQ9GOx-2A&e= 
> 
> Can you provide PCIE_GET_ATU_INB_UNR_REG_OFFSET (similar to
> PCIE_GET_ATU_OUTB_UNR_REG_OFFSET)?

Yes of course, I will send you the definition soon.

Thanks,
Joao

> 
> Thanks
> Kishon
>

[toc] | [next] | [standalone]

#1595078

FromJoao Pinto <Joao.Pinto@synopsys.com>
Date2017-03-08 12:40 +0100
Message-ID<tiI6K-5Wo-5@gated-at.bofh.it>
In reply to#1594173
Hi Kishon,

>> Can you provide PCIE_GET_ATU_INB_UNR_REG_OFFSET (similar to
>> PCIE_GET_ATU_OUTB_UNR_REG_OFFSET)?
> 
> Yes of course, I will send you the definition soon.

As promissed here is the definition for Inbound:

+/* register address builder */
+#define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
+					((0x3 << 20) | (region << 9) |	\
+					(0x1 << 8) | (register << 2))

Thanks,
Joao

> 
> Thanks,
> Joao
> 
>>
>> Thanks
>> Kishon
>>
>

[toc] | [prev] | [next] | [standalone]

#1595088

FromKishon Vijay Abraham I <kishon@ti.com>
Date2017-03-08 12:50 +0100
Message-ID<tiIgq-5ZP-9@gated-at.bofh.it>
In reply to#1595078
Hi,

On Wednesday 08 March 2017 05:02 PM, Joao Pinto wrote:
> 
> Hi Kishon,
> 
>>> Can you provide PCIE_GET_ATU_INB_UNR_REG_OFFSET (similar to
>>> PCIE_GET_ATU_OUTB_UNR_REG_OFFSET)?
>>
>> Yes of course, I will send you the definition soon.
> 
> As promissed here is the definition for Inbound:
> 
> +/* register address builder */
> +#define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
> +					((0x3 << 20) | (region << 9) |	\
> +					(0x1 << 8) | (register << 2))

Cool, thanks!

-Kishon
> 
> Thanks,
> Joao
> 
>>
>> Thanks,
>> Joao
>>
>>>
>>> Thanks
>>> Kishon
>>>
>>
>

[toc] | [prev] | [next] | [standalone]

#1595089

FromJoao Pinto <Joao.Pinto@synopsys.com>
Date2017-03-08 12:50 +0100
Message-ID<tiIgq-5ZP-19@gated-at.bofh.it>
In reply to#1595088
Às 11:35 AM de 3/8/2017, Kishon Vijay Abraham I escreveu:
> Hi,
> 
> On Wednesday 08 March 2017 05:02 PM, Joao Pinto wrote:
>>
>> Hi Kishon,
>>
>>>> Can you provide PCIE_GET_ATU_INB_UNR_REG_OFFSET (similar to
>>>> PCIE_GET_ATU_OUTB_UNR_REG_OFFSET)?
>>>
>>> Yes of course, I will send you the definition soon.
>>
>> As promissed here is the definition for Inbound:
>>
>> +/* register address builder */
>> +#define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
>> +					((0x3 << 20) | (region << 9) |	\
>> +					(0x1 << 8) | (register << 2))
> 
> Cool, thanks!

No problem! If you have doubts, please let me know.

Thanks,
Joao

> 
> -Kishon
>>
>> Thanks,
>> Joao
>>
>>>
>>> Thanks,
>>> Joao
>>>
>>>>
>>>> Thanks
>>>> Kishon
>>>>
>>>
>>

[toc] | [prev] | [next] | [standalone]

#1595284

FromJoao Pinto <Joao.Pinto@synopsys.com>
Date2017-03-08 16:40 +0100
Message-ID<tiLQZ-8we-1@gated-at.bofh.it>
In reply to#1595089
Às 1:31 PM de 3/8/2017, Kishon Vijay Abraham I escreveu:
> Hi,
> 
> On Wednesday 08 March 2017 05:07 PM, Joao Pinto wrote:
>> Às 11:35 AM de 3/8/2017, Kishon Vijay Abraham I escreveu:
>>> Hi,
>>>
>>> On Wednesday 08 March 2017 05:02 PM, Joao Pinto wrote:
>>>>
>>>> Hi Kishon,
>>>>
>>>>>> Can you provide PCIE_GET_ATU_INB_UNR_REG_OFFSET (similar to
>>>>>> PCIE_GET_ATU_OUTB_UNR_REG_OFFSET)?
>>>>>
>>>>> Yes of course, I will send you the definition soon.
>>>>
>>>> As promissed here is the definition for Inbound:
>>>>
>>>> +/* register address builder */
>>>> +#define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
>>>> +					((0x3 << 20) | (region << 9) |	\
>>>> +					(0x1 << 8) | (register << 2))
>>>
>>> Cool, thanks!
>>
>> No problem! If you have doubts, please let me know.
> 
> Okay, so this looks slightly different than the outbound macro since it takes
> the register argument. In the case of outbound PCIE_GET_ATU_OUTB_UNR_REG_OFFSET
> returns the offset which was used like
> dw_pcie_write_dbi(pci, base, offset + reg, 0x4, val);
> 
> How should the value from PCIE_GET_ATU_INB_UNR_REG_ADDR be used?

My original way was this one:

+/* Register address builder */
+#define PCIE_GET_ATU_OUTB_UNR_REG_ADDR(region, register)		\
+					((0x3 << 20) | (region << 9) |	\
+					(register << 2))

Bjorn then converted to offset:

#define PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(region)  ((0x3 << 20) | (region << 9))

and applied the <<2 shift to the ATU registers.

So you can use:

#define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
					((0x3 << 20) | (region << 9) |	\
					(0x1 << 8)

Thanks.

> 
> Thanks
> Kishon
>

[toc] | [prev] | [next] | [standalone]

#1595289

FromJoao Pinto <Joao.Pinto@synopsys.com>
Date2017-03-08 16:40 +0100
Message-ID<tiLR0-8we-25@gated-at.bofh.it>
In reply to#1595284
Às 3:32 PM de 3/8/2017, Joao Pinto escreveu:
> Às 1:31 PM de 3/8/2017, Kishon Vijay Abraham I escreveu:
>> Hi,
>>
>> On Wednesday 08 March 2017 05:07 PM, Joao Pinto wrote:
>>> Às 11:35 AM de 3/8/2017, Kishon Vijay Abraham I escreveu:
>>>> Hi,
>>>>
>>>> On Wednesday 08 March 2017 05:02 PM, Joao Pinto wrote:
>>>>>
>>>>> Hi Kishon,
>>>>>
>>>>>>> Can you provide PCIE_GET_ATU_INB_UNR_REG_OFFSET (similar to
>>>>>>> PCIE_GET_ATU_OUTB_UNR_REG_OFFSET)?
>>>>>>
>>>>>> Yes of course, I will send you the definition soon.
>>>>>
>>>>> As promissed here is the definition for Inbound:
>>>>>
>>>>> +/* register address builder */
>>>>> +#define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
>>>>> +					((0x3 << 20) | (region << 9) |	\
>>>>> +					(0x1 << 8) | (register << 2))
>>>>
>>>> Cool, thanks!
>>>
>>> No problem! If you have doubts, please let me know.
>>
>> Okay, so this looks slightly different than the outbound macro since it takes
>> the register argument. In the case of outbound PCIE_GET_ATU_OUTB_UNR_REG_OFFSET
>> returns the offset which was used like
>> dw_pcie_write_dbi(pci, base, offset + reg, 0x4, val);
>>
>> How should the value from PCIE_GET_ATU_INB_UNR_REG_ADDR be used?
> 
> My original way was this one:
> 
> +/* Register address builder */
> +#define PCIE_GET_ATU_OUTB_UNR_REG_ADDR(region, register)		\
> +					((0x3 << 20) | (region << 9) |	\
> +					(register << 2))
> 
> Bjorn then converted to offset:
> 
> #define PCIE_GET_ATU_OUTB_UNR_REG_OFFSET(region)  ((0x3 << 20) | (region << 9))
> 
> and applied the <<2 shift to the ATU registers.
> 
> So you can use:
> 
> #define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
> 					((0x3 << 20) | (region << 9) |	\
> 					(0x1 << 8)
> 

This one has the right name :)

#define PCIE_GET_ATU_INB_UNR_REG_OFFSET(region, register)		\
					((0x3 << 20) | (region << 9) |	\
					(0x1 << 8)


> Thanks.
> 
>>
>> Thanks
>> Kishon
>>
>

[toc] | [prev] | [next] | [standalone]

#1595459

FromChristoph Hellwig <hch@infradead.org>
Date2017-03-08 20:20 +0100
Message-ID<tiPhT-2rk-1@gated-at.bofh.it>
In reply to#1595284
On Wed, Mar 08, 2017 at 03:32:03PM +0000, Joao Pinto wrote:
> #define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
> 					((0x3 << 20) | (region << 9) |	\
> 					(0x1 << 8)

Can you turn this and any similar macro into inline functions?

[toc] | [prev] | [next] | [standalone]

#1595987

FromJoao Pinto <Joao.Pinto@synopsys.com>
Date2017-03-09 13:00 +0100
Message-ID<tj4TF-4Fc-33@gated-at.bofh.it>
In reply to#1595459
Hi Christoph,

Às 7:14 PM de 3/8/2017, Christoph Hellwig escreveu:
> On Wed, Mar 08, 2017 at 03:32:03PM +0000, Joao Pinto wrote:
>> #define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
>> 					((0x3 << 20) | (region << 9) |	\
>> 					(0x1 << 8)
> 
> Can you turn this and any similar macro into inline functions?
> 

Use an inline functions instead of macro is fine by me. In the initial patch I
implemented as a macro since it only does simple bit shifts and nothing else and
seemed simpler as a macro.

Thanks,
Joao

[toc] | [prev] | [next] | [standalone]

#1595318

FromKishon Vijay Abraham I <kishon@ti.com>
Date2017-03-08 17:10 +0100
Message-ID<tiLQZ-8we-3@gated-at.bofh.it>
In reply to#1595089
Hi,

On Wednesday 08 March 2017 05:07 PM, Joao Pinto wrote:
> Às 11:35 AM de 3/8/2017, Kishon Vijay Abraham I escreveu:
>> Hi,
>>
>> On Wednesday 08 March 2017 05:02 PM, Joao Pinto wrote:
>>>
>>> Hi Kishon,
>>>
>>>>> Can you provide PCIE_GET_ATU_INB_UNR_REG_OFFSET (similar to
>>>>> PCIE_GET_ATU_OUTB_UNR_REG_OFFSET)?
>>>>
>>>> Yes of course, I will send you the definition soon.
>>>
>>> As promissed here is the definition for Inbound:
>>>
>>> +/* register address builder */
>>> +#define PCIE_GET_ATU_INB_UNR_REG_ADDR(region, register)		\
>>> +					((0x3 << 20) | (region << 9) |	\
>>> +					(0x1 << 8) | (register << 2))
>>
>> Cool, thanks!
> 
> No problem! If you have doubts, please let me know.

Okay, so this looks slightly different than the outbound macro since it takes
the register argument. In the case of outbound PCIE_GET_ATU_OUTB_UNR_REG_OFFSET
returns the offset which was used like
dw_pcie_write_dbi(pci, base, offset + reg, 0x4, val);

How should the value from PCIE_GET_ATU_INB_UNR_REG_ADDR be used?

Thanks
Kishon

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web