Re: [PATCH v4 2/3] PCI: qcom: add support to msm8996 PCIE controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Srini,

On 11/15/2016 03:22 PM, Srinivas Kandagatla wrote:
> 
> 
> On 15/11/16 12:24, Stanimir Varbanov wrote:
>> Hi Srini,
>>
>> On 11/14/2016 01:15 PM, Srinivas Kandagatla wrote:
>>> This patch adds support to msm8996/apq8096 pcie, MSM8996 supports
>>> Gen 1/2, One lane, 3 pcie root-complex with support to MSI and
>>> legacy interrupts and it conforms to PCI Express Base 2.1 specification.
>>>
>>> This patch adds post_init callback to qcom_pcie_ops, as this is pcie
>>> pipe clocks are only setup after the phy is powered on.
>>> It also adds ltssm_enable callback as it is very much different to other
>>> supported SOCs in the driver.
>>>
>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
>>
>> With below comments addressed:
>>
>> Acked-by: Stanimir Varbanov <svarbanov@xxxxxxxxxx>
> Thanks for the ack.
>>
>>> ---
>>>  .../devicetree/bindings/pci/qcom,pcie.txt          |  67 +++++++-
>>>  drivers/pci/host/pcie-qcom.c                       | 177
>>> ++++++++++++++++++++-
>>>  2 files changed, 238 insertions(+), 6 deletions(-)
>>>
>>
>> <snip>
>>
>>> diff --git a/drivers/pci/host/pcie-qcom.c b/drivers/pci/host/pcie-qcom.c
>>> index 3593640..03ba6b1 100644
>>> --- a/drivers/pci/host/pcie-qcom.c
>>> +++ b/drivers/pci/host/pcie-qcom.c
>>> @@ -36,11 +36,19 @@
>>>
>>>  #include "pcie-designware.h"
>>>
>>> +#define PCIE20_PARF_DBI_BASE_ADDR    0x168
>>
>> This is already defined few rows below, please drop it.
>>
> Yep, will remove this.
>>> +
>>> +#define PCIE20_PARF_SYS_CTRL            0x00
>>>  #define PCIE20_PARF_PHY_CTRL            0x40
>>>  #define PCIE20_PARF_PHY_REFCLK            0x4C
>>>  #define PCIE20_PARF_DBI_BASE_ADDR        0x168
>>>  #define PCIE20_PARF_SLV_ADDR_SPACE_SIZE        0x16c
>>> +#define PCIE20_PARF_MHI_CLOCK_RESET_CTRL    0x174
>>>  #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT    0x178
>>> +#define MSM8996_PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT   0x1A8
>>
>> I don't like MSM8996_ prefix. Could you invent a macro which depending
>> on controller selects proper offset?
> 
> maybe some like this ??
> 
> #define PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V2    0x1A8

No, I wanted to preserve the name of the register offset. By that way in
the next pcie controller version we do not need to have
PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT_V3.

I was thinking for something like

PCIE20_PARF_AXI_MSTR_WR_ADDR_HALT(ver)	\
		((ver) == VERSION_1 ? 0x178 : 0x1A8)

But you will need to extend qcom_pcie_ops with new member to store the
version.

It's up to you ... or we can fix it when new version of the controller
appear.

regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux