[U-Boot] [PATCH] keystone2: use appropriate HD field for destination port
ivan.khoronzhuk
ivan.khoronzhuk at linaro.org
Wed Jul 8 21:22:44 CEST 2015
Vitaly,
On 08.07.15 21:28, Vitaly Andrianov wrote:
>
>
> On 07/08/2015 01:50 PM, ivan.khoronzhuk wrote:
>> Vitaly,
>>
>> On 08.07.15 20:26, Vitaly Andrianov wrote:
>>>
>>>
>>> On 07/08/2015 01:05 PM, Ivan Khoronzhuk wrote:
>>>> Vitaly,
>>>>
>>>> On 08.07.15 20:05, Vitaly Andrianov wrote:
>>>>>
>>>>>
>>>>> On 07/08/2015 12:38 PM, Ivan Khoronzhuk wrote:
>>>>>> Hi, Vitaly
>>>>>>
>>>>>> I suppose it's better to decide in upper driver how to use swinfo
>>>>>> field.
>>>>>> Like in drivers/net/keystone_net.c
>>>>>>
>>>>>> The keystone navigator supposed to be used as a tool for
>>>>>> communicating
>>>>>> between different IPs, and each of them decide how to use swinfo
>>>>>> fields.
>>>>>> It's protocol specific information and should be known only in
>>>>>> sending
>>>>>> parts. What if tomorrow you will decide to send some packet to PA?,
>>>>>> you
>>>>>> will rewrite this function again?
>>>>>>
>>>>>> It's not the place for such kind information.
>>>>>>
>>>>>> Even more, this is the h/w specific decision and no need to check
>>>>>> this
>>>>>> for each sent packet. You better statically assign how to use this
>>>>>> field
>>>>>> depending on h/w revision, using #if.
>>>>>>
>>>>>> On 08.07.15 18:45, Vitaly Andrianov wrote:
>>>>>>> K2L and L2E have different from K2HK EthSS version, which uses
>>>>>>> tag_info
>>>>>>> field for destination slave port. This commit adds the
>>>>>>> dest_port_info
>>>>>>> field
>>>>>>> to the struct pktdma_cfg, to configure which HD filed tag_info or
>>>>>>> pkt_info
>>>>>>> shall be used to configure descriptor.
>>>>>>>
>>>>>>> Before that commit the swinfo[2] was used for that purpose. Even if
>>>>>>> that
>>>>>>> worked on K2HK devices, the correct field for K2HK is the pkt_info.
>>>>>>>
>>>>>>> The netcp_send() configure appropriate HD info field depending on
>>>>>>> the
>>>>>>> direct_info of the currently using netcp.
>>>>>>>
>>>>>>> Signed-off-by: Vitaly Andrianov <vitalya at ti.com>
>>>>>>> Acked-by: Murali Karicheri <m-karicheri2 at ti.com>
>>>>>>> ---
>>>>>>> arch/arm/include/asm/ti-common/keystone_nav.h | 9 ++++++++-
>>>>>>> drivers/dma/keystone_nav.c | 12 ++++++++++--
>>>>>>> drivers/net/keystone_net.c | 3 +--
>>>>>>> 3 files changed, 19 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/include/asm/ti-common/keystone_nav.h
>>>>>>> b/arch/arm/include/asm/ti-common/keystone_nav.h
>>>>>>> index 696d8c6..5a0e391 100644
>>>>>>> --- a/arch/arm/include/asm/ti-common/keystone_nav.h
>>>>>>> +++ b/arch/arm/include/asm/ti-common/keystone_nav.h
>>>>>>> @@ -152,6 +152,11 @@ struct rx_flow_regs {
>>>>>>> u32 thresh[3];
>>>>>>> };
>>>>>>>
>>>>>>> +enum dest_port_info {
>>>>>>> + PKT_INFO,
>>>>>>> + TAG_INFO
>>>>>>> +};
>>>>>>> +
>>>>>>> struct pktdma_cfg {
>>>>>>> struct global_ctl_regs *global;
>>>>>>> struct tx_chan_regs *tx_ch;
>>>>>>> @@ -167,6 +172,7 @@ struct pktdma_cfg {
>>>>>>> u32 tx_snd_q;
>>>>>>>
>>>>>>> u32 rx_flow; /* flow that is used for RX */
>>>>>>> + enum dest_port_info dest_port_info;/* HD fiels for dest
>>>>>>> port
>>>>>>> bits */
>>>>>>> };
>>>>>>>
>>>>>>> extern struct pktdma_cfg netcp_pktdma;
>>>>>>> @@ -184,7 +190,8 @@ struct rx_buff_desc {
>>>>>>>
>>>>>>> int ksnav_close(struct pktdma_cfg *pktdma);
>>>>>>> int ksnav_init(struct pktdma_cfg *pktdma, struct rx_buff_desc
>>>>>>> *rx_buffers);
>>>>>>> -int ksnav_send(struct pktdma_cfg *pktdma, u32 *pkt, int num_bytes,
>>>>>>> u32 swinfo2);
>>>>>>> +int ksnav_send(struct pktdma_cfg *pktdma, u32 *pkt, int num_bytes,
>>>>>>> + u32 dest_port);
>>>>>>> void *ksnav_recv(struct pktdma_cfg *pktdma, u32 **pkt, int
>>>>>>> *num_bytes);
>>>>>>> void ksnav_release_rxhd(struct pktdma_cfg *pktdma, void *hd);
>>>>>>>
>>>>>>> diff --git a/drivers/dma/keystone_nav.c b/drivers/dma/keystone_nav.c
>>>>>>> index dfca75a..64b1cee 100644
>>>>>>> --- a/drivers/dma/keystone_nav.c
>>>>>>> +++ b/drivers/dma/keystone_nav.c
>>>>>>> @@ -278,7 +278,8 @@ int ksnav_close(struct pktdma_cfg *pktdma)
>>>>>>> return QM_OK;
>>>>>>> }
>>>>>>>
>>>>>>> -int ksnav_send(struct pktdma_cfg *pktdma, u32 *pkt, int num_bytes,
>>>>>>> u32 swinfo2)
>>>>>>> +int ksnav_send(struct pktdma_cfg *pktdma, u32 *pkt, int num_bytes,
>>>>>>> + u32 dest_port)
>>>>>>> {
>>>>>>> struct qm_host_desc *hd;
>>>>>>>
>>>>>>> @@ -286,8 +287,15 @@ int ksnav_send(struct pktdma_cfg *pktdma, u32
>>>>>>> *pkt, int num_bytes, u32 swinfo2)
>>>>>>> if (hd == NULL)
>>>>>>> return QM_ERR;
>>>>>>>
>>>>>>> + dest_port &= 0xf;
>>>>>>> hd->desc_info = num_bytes;
>>>>>>> - hd->swinfo[2] = swinfo2;
>>>>>>> + if (pktdma->dest_port_info == PKT_INFO) {
>>>>>>> + hd->packet_info = qm_cfg->qpool_num | (dest_port << 16);
>>>>>>> + } else {
>>>>>>> + hd->packet_info = qm_cfg->qpool_num;
>>>>>>> + hd->tag_info = dest_port;
>>>>>>> + }
>>>>>>> +
>>>>>>> hd->packet_info = qm_cfg->qpool_num;
>>>>>>>
>>>>>>> qm_buff_push(hd, pktdma->tx_snd_q, pkt, num_bytes);
>>>>>>> diff --git a/drivers/net/keystone_net.c b/drivers/net/keystone_net.c
>>>>>>> index 0c5fdee..e2adb67 100644
>>>>>>> --- a/drivers/net/keystone_net.c
>>>>>>> +++ b/drivers/net/keystone_net.c
>>>>>>> @@ -381,8 +381,7 @@ int32_t cpmac_drv_send(u32 *buffer, int
>>>>>>> num_bytes,
>>>>>>> int slave_port_num)
>>>>>>> if (num_bytes < EMAC_MIN_ETHERNET_PKT_SIZE)
>>>>>>> num_bytes = EMAC_MIN_ETHERNET_PKT_SIZE;
>>>>>>>
>>>>>>> - return ksnav_send(&netcp_pktdma, buffer,
>>>>>>> - num_bytes, (slave_port_num) << 16);
>>>>>>> + return ksnav_send(&netcp_pktdma, buffer, num_bytes,
>>>>>>> slave_port_num);
>>>>>>> }
>>>>>>>
>>>>>>> /* Eth device open */
>>>>>>>
>>>>>>
>>>>> Hi Ivan,
>>>>>
>>>>> I agree with you. And probably we will need to implement your proposal
>>>>> in future commits. This commit is to fix the bug, which is in existing
>>>>> driver.
>>>>>
>>>>> Thanks,
>>>>> Vitaly
>>>>
>>>> Sorry, I supposed that first msg was not sent.
>>>> It's better to fix it in drivers/net/keystone_net.c
>>>>
>>>>
>>> Ivan,
>>>
>>> I don't understand your proposal. The network driver doesn't know
>>> anything about CPPI DMA engine internals. That is the navigator's job to
>>> fill appropriate packet BD fields and it only takes care about HW
>>> version differences.
>>
>> You have Eth h/w, you have pktDMA engine to communicate through, you
>> have network driver that knows how to use pktDMA to communicate with
>> Ethernet switch.
>> CPII DMA engine can be used to communicate with different h/w, not only
>> with Ethernet switch. This can be used to send packets to PA, to SA, to
>> QMSS PDSPs, to PA PDSPs and others parts, I don't talk even about AIF,
>> SRIO, FFTC and others IPs that have Pktdma on board.
>>
>> Each of them can use swinfo for different purposes.
>>
>> In the chain to communicate directly with Eth you use net driver
>> in the way keystone_net -> keystone_nav -> h/w pktdma -> eht
>> You can use it also like keystone_srio -> keystone_nav -> h/w pktdma ->
>> srio.
>> etc.
>>
>> pktdma engine it's like a bus used to communicate with a h/w. So, it's
>> not correct to fix driver specific issues in the bus s/w, that is used
>> by different drivers.
>>
> OK. What is your proposal? How to fix the issue in the keystone_net.c,
> which doesn't have any idea about structure of buffer descriptors. And
> actually doesn't have to know about it.
>
> The proposed patch fixes the bug of existing upstreamed driver and allow
> K2E and K2L users to work. It is fully tested on all supported devices.
>
> The PA PDSP and SA drivers currently doesn't exist in u-boot and I don't
> have any information whether anybody is going to develop them in the
> nearest future.
>
It doesn't matter exist those drivers or not. It's a structured model.
If, for instance, you are going to fix some errata in I2C RTC, what
will you do? fix it in I2C driver? But better place is RTC driver.
It's partly an API issue. It should be allowed to assign protocol
specific flags field in packet_info and allow others to use swinfo in
the same time.
It be better to add to pktdma_config field like protocol_flags.
in keystone_nav send function do:
hd->packet_info = qm_cfg->qpool_num |
(qm_cfg->protocol_flags) & 0xf << 16;
and in keystone_net, statically:
#if (REVISION == REVISION_WITH_BUG)
qm_cfg->protocol_flags = dest_port;
#else
qm_cfg->protocol_flags = 0;
But I'm not sure about one thing. Do you have revisions that use swinfo
for port number and cannot use packet_info for this goal? In this case
you should support version when software field is used also.
>>>
>>> Thanks,
>>> Vitaly
>>> _______________________________________________
>>> U-Boot mailing list
>>> U-Boot at lists.denx.de
>>> http://lists.denx.de/mailman/listinfo/u-boot
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list