diff mbox

PCI: designware: Fix PORT_LOGIC_LINK_WIDTH_MASK

Message ID 1440498929-44519-1-git-send-email-wangzhou1@hisilicon.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Zhou Wang Aug. 25, 2015, 10:35 a.m. UTC
The value under PORT_LOGIC_LINK_WIDTH_MASK is 0x1, 0x2, 0x4, 0x8. Here change
this mask to proper value.

In fact, for DesignWare PCIe IP version 4.4, it only uses bit8~12 to indicate
number of lanes. Original mask will bring a mistake.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/host/pcie-designware.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Han Jingoo Aug. 25, 2015, 11:14 a.m. UTC | #1
On 2015. 8. 25., at PM 7:35, Zhou Wang <wangzhou1@hisilicon.com> wrote:
> 
> The value under PORT_LOGIC_LINK_WIDTH_MASK is 0x1, 0x2, 0x4, 0x8. Here change
> this mask to proper value.
> 
> In fact, for DesignWare PCIe IP version 4.4, it only uses bit8~12 to indicate
> number of lanes. Original mask will bring a mistake.

NAK

The definitions of bits of registers should be defined based on hardware manual such as datasheet, databook.

According to the databook, the bits [16:8] are defined for NUM_OF_LANES.

In the future, if 16 lanes or 32 lanes is supported and 12th bit or 13th bit is used, what will you do?

Then, in the version 4.4 IP, the bits[16:13] are used for other usage?

Best regards,
Jingoo Han

> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
> drivers/pci/host/pcie-designware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 69486be..eb549b9 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -35,7 +35,7 @@
> 
> #define PCIE_LINK_WIDTH_SPEED_CONTROL    0x80C
> #define PORT_LOGIC_SPEED_CHANGE        (0x1 << 17)
> -#define PORT_LOGIC_LINK_WIDTH_MASK    (0x1ff << 8)
> +#define PORT_LOGIC_LINK_WIDTH_MASK    (0x1f << 8)
> #define PORT_LOGIC_LINK_WIDTH_1_LANES    (0x1 << 8)
> #define PORT_LOGIC_LINK_WIDTH_2_LANES    (0x2 << 8)
> #define PORT_LOGIC_LINK_WIDTH_4_LANES    (0x4 << 8)
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhou Wang Aug. 26, 2015, 1:57 a.m. UTC | #2
Hi Jingoo,

On 2015/8/25 19:14, Jingoo Han wrote:
> On 2015. 8. 25., at PM 7:35, Zhou Wang <wangzhou1@hisilicon.com> wrote:
>>
>> The value under PORT_LOGIC_LINK_WIDTH_MASK is 0x1, 0x2, 0x4, 0x8. Here change
>> this mask to proper value.
>>
>> In fact, for DesignWare PCIe IP version 4.4, it only uses bit8~12 to indicate
>> number of lanes. Original mask will bring a mistake.
>
> NAK

Sorry, maybe above commit message is no very clear. Will explain it below.

> 
> The definitions of bits of registers should be defined based on hardware manual such as datasheet, databook.
> 
> According to the databook, the bits [16:8] are defined for NUM_OF_LANES.

Yes, according to databook of v4.2, bits [16:8] are defined for NUM_OF_LANES.

> 
> In the future, if 16 lanes or 32 lanes is supported and 12th bit or 13th bit is used, what will you do?
> 
> Then, in the version 4.4 IP, the bits[16:13] are used for other usage?
>

Yes, according to databook of v4.4, bits [12:8] are defined for NUM_OF_LANES.
[16:13] are for other usages(bit16 is AUTO_LANE_FLIP_CTRL_EN, bits [15:13] are
PRE_DET_LANE). For details, maybe you can refer to v4.4 databook :)

My opinion is that there is no conflict about NUM_OF_LANES between v4.2 and v4.4.
We can change this mask to avoid future problem.

Thanks
Zhou

> Best regards,
> Jingoo Han
> 
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>> ---
>> drivers/pci/host/pcie-designware.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>> index 69486be..eb549b9 100644
>> --- a/drivers/pci/host/pcie-designware.c
>> +++ b/drivers/pci/host/pcie-designware.c
>> @@ -35,7 +35,7 @@
>>
>> #define PCIE_LINK_WIDTH_SPEED_CONTROL    0x80C
>> #define PORT_LOGIC_SPEED_CHANGE        (0x1 << 17)
>> -#define PORT_LOGIC_LINK_WIDTH_MASK    (0x1ff << 8)
>> +#define PORT_LOGIC_LINK_WIDTH_MASK    (0x1f << 8)
>> #define PORT_LOGIC_LINK_WIDTH_1_LANES    (0x1 << 8)
>> #define PORT_LOGIC_LINK_WIDTH_2_LANES    (0x2 << 8)
>> #define PORT_LOGIC_LINK_WIDTH_4_LANES    (0x4 << 8)
>> -- 
>> 1.9.1
>>
> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Han Jingoo Aug. 26, 2015, 2:17 a.m. UTC | #3
On 2015. 8. 26., at AM 10:57, Zhou Wang <wangzhou1@hisilicon.com> wrote:
> 
> Hi Jingoo,
> 
>> On 2015/8/25 19:14, Jingoo Han wrote:
>>> On 2015. 8. 25., at PM 7:35, Zhou Wang <wangzhou1@hisilicon.com> wrote:
>>> 
>>> The value under PORT_LOGIC_LINK_WIDTH_MASK is 0x1, 0x2, 0x4, 0x8. Here change
>>> this mask to proper value.
>>> 
>>> In fact, for DesignWare PCIe IP version 4.4, it only uses bit8~12 to indicate
>>> number of lanes. Original mask will bring a mistake.
>> 
>> NAK
> 
> Sorry, maybe above commit message is no very clear. Will explain it below.
> 
>> 
>> The definitions of bits of registers should be defined based on hardware manual such as datasheet, databook.
>> 
>> According to the databook, the bits [16:8] are defined for NUM_OF_LANES.
> 
> Yes, according to databook of v4.2, bits [16:8] are defined for NUM_OF_LANES.
> 
>> 
>> In the future, if 16 lanes or 32 lanes is supported and 12th bit or 13th bit is used, what will you do?
>> 
>> Then, in the version 4.4 IP, the bits[16:13] are used for other usage?
> 
> Yes, according to databook of v4.4, bits [12:8] are defined for NUM_OF_LANES.
> [16:13] are for other usages(bit16 is AUTO_LANE_FLIP_CTRL_EN, bits [15:13] are
> PRE_DET_LANE). For details, maybe you can refer to v4.4 databook :)
> 
> My opinion is that there is no conflict about NUM_OF_LANES between v4.2 and v4.4.
> We can change this mask to avoid future problem.

OK, I see.
I got what you want to say. :-)

I think that there is no conflict between v4.2 and v4.4.
So, this patch can be accepted.

Then, would you send v2 patch with more detailed commit message above mentioned?

Best regards,
Jingoo Han

> Thanks
> Zhou
> 
>> Best regards,
>> Jingoo Han
>> 
>>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>>> ---
>>> drivers/pci/host/pcie-designware.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>>> index 69486be..eb549b9 100644
>>> --- a/drivers/pci/host/pcie-designware.c
>>> +++ b/drivers/pci/host/pcie-designware.c
>>> @@ -35,7 +35,7 @@
>>> 
>>> #define PCIE_LINK_WIDTH_SPEED_CONTROL    0x80C
>>> #define PORT_LOGIC_SPEED_CHANGE (0x1 << 17)
>>> -#define PORT_LOGIC_LINK_WIDTH_MASK    (0x1ff << 8)
>>> +#define PORT_LOGIC_LINK_WIDTH_MASK    (0x1f << 8)
>>> #define PORT_LOGIC_LINK_WIDTH_1_LANES    (0x1 << 8)
>>> #define PORT_LOGIC_LINK_WIDTH_2_LANES    (0x2 << 8)
>>> #define PORT_LOGIC_LINK_WIDTH_4_LANES    (0x4 << 8)
>>> -- 
>>> 1.9.1
>> 
>> .
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhou Wang Aug. 26, 2015, 2:47 a.m. UTC | #4
On 2015/8/26 10:17, Jingoo Han wrote:
> On 2015. 8. 26., at AM 10:57, Zhou Wang <wangzhou1@hisilicon.com> wrote:
>>
>> Hi Jingoo,
>>
>>> On 2015/8/25 19:14, Jingoo Han wrote:
>>>> On 2015. 8. 25., at PM 7:35, Zhou Wang <wangzhou1@hisilicon.com> wrote:
>>>>
>>>> The value under PORT_LOGIC_LINK_WIDTH_MASK is 0x1, 0x2, 0x4, 0x8. Here change
>>>> this mask to proper value.
>>>>
>>>> In fact, for DesignWare PCIe IP version 4.4, it only uses bit8~12 to indicate
>>>> number of lanes. Original mask will bring a mistake.
>>>
>>> NAK
>>
>> Sorry, maybe above commit message is no very clear. Will explain it below.
>>
>>>
>>> The definitions of bits of registers should be defined based on hardware manual such as datasheet, databook.
>>>
>>> According to the databook, the bits [16:8] are defined for NUM_OF_LANES.
>>
>> Yes, according to databook of v4.2, bits [16:8] are defined for NUM_OF_LANES.
>>
>>>
>>> In the future, if 16 lanes or 32 lanes is supported and 12th bit or 13th bit is used, what will you do?
>>>
>>> Then, in the version 4.4 IP, the bits[16:13] are used for other usage?
>>
>> Yes, according to databook of v4.4, bits [12:8] are defined for NUM_OF_LANES.
>> [16:13] are for other usages(bit16 is AUTO_LANE_FLIP_CTRL_EN, bits [15:13] are
>> PRE_DET_LANE). For details, maybe you can refer to v4.4 databook :)
>>
>> My opinion is that there is no conflict about NUM_OF_LANES between v4.2 and v4.4.
>> We can change this mask to avoid future problem.
> 
> OK, I see.
> I got what you want to say. :-)
> 
> I think that there is no conflict between v4.2 and v4.4.
> So, this patch can be accepted.
> 
> Then, would you send v2 patch with more detailed commit message above mentioned?
>

Thanks. I will prepare v2 patch with more detailed commit message.

Best regards,
Zhou

> Best regards,
> Jingoo Han
>
>> Thanks
>> Zhou
>>
>>> Best regards,
>>> Jingoo Han
>>>
>>>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>>>> ---
>>>> drivers/pci/host/pcie-designware.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
>>>> index 69486be..eb549b9 100644
>>>> --- a/drivers/pci/host/pcie-designware.c
>>>> +++ b/drivers/pci/host/pcie-designware.c
>>>> @@ -35,7 +35,7 @@
>>>>
>>>> #define PCIE_LINK_WIDTH_SPEED_CONTROL    0x80C
>>>> #define PORT_LOGIC_SPEED_CHANGE (0x1 << 17)
>>>> -#define PORT_LOGIC_LINK_WIDTH_MASK    (0x1ff << 8)
>>>> +#define PORT_LOGIC_LINK_WIDTH_MASK    (0x1f << 8)
>>>> #define PORT_LOGIC_LINK_WIDTH_1_LANES    (0x1 << 8)
>>>> #define PORT_LOGIC_LINK_WIDTH_2_LANES    (0x2 << 8)
>>>> #define PORT_LOGIC_LINK_WIDTH_4_LANES    (0x4 << 8)
>>>> -- 
>>>> 1.9.1
>>>
>>> .
>>
>>
> 
> .
> 


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

Patch

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 69486be..eb549b9 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -35,7 +35,7 @@ 
 
 #define PCIE_LINK_WIDTH_SPEED_CONTROL	0x80C
 #define PORT_LOGIC_SPEED_CHANGE		(0x1 << 17)
-#define PORT_LOGIC_LINK_WIDTH_MASK	(0x1ff << 8)
+#define PORT_LOGIC_LINK_WIDTH_MASK	(0x1f << 8)
 #define PORT_LOGIC_LINK_WIDTH_1_LANES	(0x1 << 8)
 #define PORT_LOGIC_LINK_WIDTH_2_LANES	(0x2 << 8)
 #define PORT_LOGIC_LINK_WIDTH_4_LANES	(0x4 << 8)