diff mbox series

[1/6] net: macb: use dummy descriptor for RBQP

Message ID 1606987556-20217-2-git-send-email-claudiu.beznea@microchip.com
State Superseded
Delegated to: Eugen Hristev
Headers show
Series add support for sama7g5 ethernet interfaces | expand

Commit Message

Claudiu Beznea Dec. 3, 2020, 9:25 a.m. UTC
In case of multiple queues on RX side the queue scheduler
will try to use all the available configured queues (with
descriptors having TX_USED bit cleared). If at least one RBQP
points to a descriptor with a valid used bit configuration then
the reception may block as this may point to any memory. To avoid
this scenario all the queues (except queue zero) were disabled by
setting DMA descriptors with used bit set on proper RBQP. The driver
anyway uses only queue 0 for TX/RX.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/net/macb.c | 4 +++-
 drivers/net/macb.h | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Eugen Hristev Dec. 16, 2020, 6:54 a.m. UTC | #1
On 03.12.2020 11:25, Claudiu Beznea wrote:
> In case of multiple queues on RX side the queue scheduler
> will try to use all the available configured queues (with
> descriptors having TX_USED bit cleared). If at least one RBQP
> points to a descriptor with a valid used bit configuration then
> the reception may block as this may point to any memory. To avoid
> this scenario all the queues (except queue zero) were disabled by
> setting DMA descriptors with used bit set on proper RBQP. The driver
> anyway uses only queue 0 for TX/RX.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---

Hi Anup, Bin, Padmarao,

I noticed on the mailing list that you have been actively working and 
testing the Macb driver on various platforms, we have this series 
outstanding and I want to make sure that it does not break anything on 
your side, so it would be appreciated if you could have a look or test 
it before it goes into master branch.

Thanks !
Eugen


>   drivers/net/macb.c | 4 +++-
>   drivers/net/macb.h | 2 ++
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index b80a259ff757..836eb85ec96a 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -732,8 +732,10 @@ static int gmac_init_multi_queues(struct macb_device *macb)
>   	flush_dcache_range(macb->dummy_desc_dma, macb->dummy_desc_dma +
>   			ALIGN(MACB_TX_DUMMY_DMA_DESC_SIZE, PKTALIGN));
>   
> -	for (i = 1; i < num_queues; i++)
> +	for (i = 1; i < num_queues; i++) {
>   		gem_writel_queue_TBQP(macb, macb->dummy_desc_dma, i - 1);
> +		gem_writel_queue_RBQP(macb, macb->dummy_desc_dma, i - 1);
> +	}
>   
>   	return 0;
>   }
> diff --git a/drivers/net/macb.h b/drivers/net/macb.h
> index 9b16383eba46..28c7fe306883 100644
> --- a/drivers/net/macb.h
> +++ b/drivers/net/macb.h
> @@ -768,5 +768,7 @@
>   #define GEM_RX_CSUM_CHECKED_MASK		2
>   #define gem_writel_queue_TBQP(port, value, queue_num)	\
>   	writel((value), (port)->regs + GEM_TBQP(queue_num))
> +#define gem_writel_queue_RBQP(port, value, queue_num)	\
> +	writel((value), (port)->regs + GEM_RBQP(queue_num))
>   
>   #endif /* __DRIVERS_MACB_H__ */
>
Bin Meng Dec. 16, 2020, 7:17 a.m. UTC | #2
Hi Eugen,

On Wed, Dec 16, 2020 at 2:55 PM <Eugen.Hristev@microchip.com> wrote:
>
> On 03.12.2020 11:25, Claudiu Beznea wrote:
> > In case of multiple queues on RX side the queue scheduler
> > will try to use all the available configured queues (with
> > descriptors having TX_USED bit cleared). If at least one RBQP
> > points to a descriptor with a valid used bit configuration then
> > the reception may block as this may point to any memory. To avoid
> > this scenario all the queues (except queue zero) were disabled by
> > setting DMA descriptors with used bit set on proper RBQP. The driver
> > anyway uses only queue 0 for TX/RX.
> >
> > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> > ---
>
> Hi Anup, Bin, Padmarao,
>
> I noticed on the mailing list that you have been actively working and
> testing the Macb driver on various platforms, we have this series
> outstanding and I want to make sure that it does not break anything on
> your side, so it would be appreciated if you could have a look or test
> it before it goes into master branch.
>

Is this series for Microchip PolarFire SoC?

Regards,
Bin
Eugen Hristev Dec. 16, 2020, 8:29 a.m. UTC | #3
On 16.12.2020 09:17, Bin Meng wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Eugen,
> 
> On Wed, Dec 16, 2020 at 2:55 PM <Eugen.Hristev@microchip.com> wrote:
>>
>> On 03.12.2020 11:25, Claudiu Beznea wrote:
>>> In case of multiple queues on RX side the queue scheduler
>>> will try to use all the available configured queues (with
>>> descriptors having TX_USED bit cleared). If at least one RBQP
>>> points to a descriptor with a valid used bit configuration then
>>> the reception may block as this may point to any memory. To avoid
>>> this scenario all the queues (except queue zero) were disabled by
>>> setting DMA descriptors with used bit set on proper RBQP. The driver
>>> anyway uses only queue 0 for TX/RX.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>
>> Hi Anup, Bin, Padmarao,
>>
>> I noticed on the mailing list that you have been actively working and
>> testing the Macb driver on various platforms, we have this series
>> outstanding and I want to make sure that it does not break anything on
>> your side, so it would be appreciated if you could have a look or test
>> it before it goes into master branch.
>>
> 
> Is this series for Microchip PolarFire SoC?

No, but this series affects the macb driver, and because other platforms 
use it, I do not wish to have a chance to break them.
> 
> Regards,
> Bin
>
Padmarao Begari Dec. 17, 2020, 5:22 a.m. UTC | #4
Hi Eugen,

This series of patches break my side of work(patches) so you need to create patches after my patches are going into master branch because my patches are already reviewed and tested.

Regards
Padmarao
Eugen Hristev Jan. 14, 2021, 11:19 a.m. UTC | #5
On 17.12.2020 07:22, Padmarao Begari - I30397 wrote:
> Hi Eugen,
> 
> This series of patches break my side of work(patches) so you need to 
> create patches after my patches are going into master branch because my 
> patches are already reviewed and tested.

Hi,

Could you please detail the breakage ?
I saw a pull request with your patches that was NAK-ed, if your two macb 
patches are tested and reviewed I could apply them to the atmel tree as 
well and send them, if your PR is delayed. But we are interested to have 
our sama7g5 series pushed as well, so we need to know if it's ok on your 
side, and what is wrong with the sama7g5 series.

Thanks!
Eugen
> 
> Regards
> Padmarao
> ------------------------------------------------------------------------
> *From:* Eugen Hristev - M18282 <Eugen.Hristev@microchip.com>
> *Sent:* Wednesday, December 16, 2020 12:24 PM
> *To:* anup.patel@wdc.com <anup.patel@wdc.com>; bin.meng@windriver.com 
> <bin.meng@windriver.com>; Padmarao Begari - I30397 
> <Padmarao.Begari@microchip.com>
> *Cc:* Claudiu Beznea - M18063 <Claudiu.Beznea@microchip.com>; 
> joe.hershberger@ni.com <joe.hershberger@ni.com>; u-boot@lists.denx.de 
> <u-boot@lists.denx.de>
> *Subject:* Re: [PATCH 1/6] net: macb: use dummy descriptor for RBQP
> On 03.12.2020 11:25, Claudiu Beznea wrote:
>> In case of multiple queues on RX side the queue scheduler
>> will try to use all the available configured queues (with
>> descriptors having TX_USED bit cleared). If at least one RBQP
>> points to a descriptor with a valid used bit configuration then
>> the reception may block as this may point to any memory. To avoid
>> this scenario all the queues (except queue zero) were disabled by
>> setting DMA descriptors with used bit set on proper RBQP. The driver
>> anyway uses only queue 0 for TX/RX.
>> 
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
> 
> Hi Anup, Bin, Padmarao,
> 
> I noticed on the mailing list that you have been actively working and
> testing the Macb driver on various platforms, we have this series
> outstanding and I want to make sure that it does not break anything on
> your side, so it would be appreciated if you could have a look or test
> it before it goes into master branch.
> 
> Thanks !
> Eugen
> 
> 
>>   drivers/net/macb.c | 4 +++-
>>   drivers/net/macb.h | 2 ++
>>   2 files changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
>> index b80a259ff757..836eb85ec96a 100644
>> --- a/drivers/net/macb.c
>> +++ b/drivers/net/macb.c
>> @@ -732,8 +732,10 @@ static int gmac_init_multi_queues(struct macb_device *macb)
>>        flush_dcache_range(macb->dummy_desc_dma, macb->dummy_desc_dma +
>>                        ALIGN(MACB_TX_DUMMY_DMA_DESC_SIZE, PKTALIGN));
>>   
>> -     for (i = 1; i < num_queues; i++)
>> +     for (i = 1; i < num_queues; i++) {
>>                gem_writel_queue_TBQP(macb, macb->dummy_desc_dma, i - 1);
>> +             gem_writel_queue_RBQP(macb, macb->dummy_desc_dma, i - 1);
>> +     }
>>   
>>        return 0;
>>   }
>> diff --git a/drivers/net/macb.h b/drivers/net/macb.h
>> index 9b16383eba46..28c7fe306883 100644
>> --- a/drivers/net/macb.h
>> +++ b/drivers/net/macb.h
>> @@ -768,5 +768,7 @@
>>   #define GEM_RX_CSUM_CHECKED_MASK            2
>>   #define gem_writel_queue_TBQP(port, value, queue_num)       \
>>        writel((value), (port)->regs + GEM_TBQP(queue_num))
>> +#define gem_writel_queue_RBQP(port, value, queue_num)        \
>> +     writel((value), (port)->regs + GEM_RBQP(queue_num))
>>   
>>   #endif /* __DRIVERS_MACB_H__ */
>> 
>
Padmarao Begari Jan. 15, 2021, 4:02 a.m. UTC | #6
Hi Eugen,

On Thu, Jan 14, 2021 at 4:50 PM <Eugen.Hristev@microchip.com> wrote:

> On 17.12.2020 07:22, Padmarao Begari - I30397 wrote:
> > Hi Eugen,
> >
> > This series of patches break my side of work(patches) so you need to
> > create patches after my patches are going into master branch because my
> > patches are already reviewed and tested.
>
> Hi,
>
> Could you please detail the breakage ?
>

The breakage is the fdt relocation disabled in the board environment
variables so I have removed it and enabled fdt relocation in PATCH v9.

Regards
Padmarao


> I saw a pull request with your patches that was NAK-ed, if your two macb
> patches are tested and reviewed I could apply them to the atmel tree as
> well and send them, if your PR is delayed. But we are interested to have
> our sama7g5 series pushed as well, so we need to know if it's ok on your
> side, and what is wrong with the sama7g5 series.
>
> Thanks!
> Eugen
> >
> > Regards
> > Padmarao
> > ------------------------------------------------------------------------
> > *From:* Eugen Hristev - M18282 <Eugen.Hristev@microchip.com>
> > *Sent:* Wednesday, December 16, 2020 12:24 PM
> > *To:* anup.patel@wdc.com <anup.patel@wdc.com>; bin.meng@windriver.com
> > <bin.meng@windriver.com>; Padmarao Begari - I30397
> > <Padmarao.Begari@microchip.com>
> > *Cc:* Claudiu Beznea - M18063 <Claudiu.Beznea@microchip.com>;
> > joe.hershberger@ni.com <joe.hershberger@ni.com>; u-boot@lists.denx.de
> > <u-boot@lists.denx.de>
> > *Subject:* Re: [PATCH 1/6] net: macb: use dummy descriptor for RBQP
> > On 03.12.2020 11:25, Claudiu Beznea wrote:
> >> In case of multiple queues on RX side the queue scheduler
> >> will try to use all the available configured queues (with
> >> descriptors having TX_USED bit cleared). If at least one RBQP
> >> points to a descriptor with a valid used bit configuration then
> >> the reception may block as this may point to any memory. To avoid
> >> this scenario all the queues (except queue zero) were disabled by
> >> setting DMA descriptors with used bit set on proper RBQP. The driver
> >> anyway uses only queue 0 for TX/RX.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> >> ---
> >
> > Hi Anup, Bin, Padmarao,
> >
> > I noticed on the mailing list that you have been actively working and
> > testing the Macb driver on various platforms, we have this series
> > outstanding and I want to make sure that it does not break anything on
> > your side, so it would be appreciated if you could have a look or test
> > it before it goes into master branch.
> >
> > Thanks !
> > Eugen
> >
> >
> >>   drivers/net/macb.c | 4 +++-
> >>   drivers/net/macb.h | 2 ++
> >>   2 files changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> >> index b80a259ff757..836eb85ec96a 100644
> >> --- a/drivers/net/macb.c
> >> +++ b/drivers/net/macb.c
> >> @@ -732,8 +732,10 @@ static int gmac_init_multi_queues(struct
> macb_device *macb)
> >>        flush_dcache_range(macb->dummy_desc_dma, macb->dummy_desc_dma +
> >>                        ALIGN(MACB_TX_DUMMY_DMA_DESC_SIZE, PKTALIGN));
> >>
> >> -     for (i = 1; i < num_queues; i++)
> >> +     for (i = 1; i < num_queues; i++) {
> >>                gem_writel_queue_TBQP(macb, macb->dummy_desc_dma, i - 1);
> >> +             gem_writel_queue_RBQP(macb, macb->dummy_desc_dma, i - 1);
> >> +     }
> >>
> >>        return 0;
> >>   }
> >> diff --git a/drivers/net/macb.h b/drivers/net/macb.h
> >> index 9b16383eba46..28c7fe306883 100644
> >> --- a/drivers/net/macb.h
> >> +++ b/drivers/net/macb.h
> >> @@ -768,5 +768,7 @@
> >>   #define GEM_RX_CSUM_CHECKED_MASK            2
> >>   #define gem_writel_queue_TBQP(port, value, queue_num)       \
> >>        writel((value), (port)->regs + GEM_TBQP(queue_num))
> >> +#define gem_writel_queue_RBQP(port, value, queue_num)        \
> >> +     writel((value), (port)->regs + GEM_RBQP(queue_num))
> >>
> >>   #endif /* __DRIVERS_MACB_H__ */
> >>
> >
>
>
Eugen Hristev Jan. 15, 2021, 8:04 a.m. UTC | #7
On 15.01.2021 06:02, Padmarao Begari wrote:
> Hi Eugen,
> 
> On Thu, Jan 14, 2021 at 4:50 PM <Eugen.Hristev@microchip.com 
> <mailto:Eugen.Hristev@microchip.com>> wrote:
> 
>     On 17.12.2020 07:22, Padmarao Begari - I30397 wrote:
>      > Hi Eugen,
>      >
>      > This series of patches break my side of work(patches) so you need to
>      > create patches after my patches are going into master branch
>     because my
>      > patches are already reviewed and tested.
> 
>     Hi,
> 
>     Could you please detail the breakage ?
> 
> 
> The breakage is the fdt relocation disabled in the board environment 
> variables so I have removed it and enabled fdt relocation in PATCH v9.

Maybe you misunderstand my question. I was asking about the sama7g5 macb 
series, which you claimed that breaks your current patch set.
This is a link to the series :
https://patchwork.ozlabs.org/project/uboot/list/?series=218367

Since you claimed that this series breaks your series, I am asking what 
exactly is the breakage. How does the fdt relocation in your board 
environment has anything to do with macb and these patches which are not 
applied ?

Thanks,
Eugen

> 
> Regards
> Padmarao
> 
>     I saw a pull request with your patches that was NAK-ed, if your two
>     macb
>     patches are tested and reviewed I could apply them to the atmel tree as
>     well and send them, if your PR is delayed. But we are interested to
>     have
>     our sama7g5 series pushed as well, so we need to know if it's ok on
>     your
>     side, and what is wrong with the sama7g5 series.
> 
>     Thanks!
>     Eugen
>      >
>      > Regards
>      > Padmarao
>      >
>     ------------------------------------------------------------------------
>      > *From:* Eugen Hristev - M18282 <Eugen.Hristev@microchip.com
>     <mailto:Eugen.Hristev@microchip.com>>
>      > *Sent:* Wednesday, December 16, 2020 12:24 PM
>      > *To:* anup.patel@wdc.com <mailto:anup.patel@wdc.com>
>     <anup.patel@wdc.com <mailto:anup.patel@wdc.com>>;
>     bin.meng@windriver.com <mailto:bin.meng@windriver.com>
>      > <bin.meng@windriver.com <mailto:bin.meng@windriver.com>>;
>     Padmarao Begari - I30397
>      > <Padmarao.Begari@microchip.com
>     <mailto:Padmarao.Begari@microchip.com>>
>      > *Cc:* Claudiu Beznea - M18063 <Claudiu.Beznea@microchip.com
>     <mailto:Claudiu.Beznea@microchip.com>>;
>      > joe.hershberger@ni.com <mailto:joe.hershberger@ni.com>
>     <joe.hershberger@ni.com <mailto:joe.hershberger@ni.com>>;
>     u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>
>      > <u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>>
>      > *Subject:* Re: [PATCH 1/6] net: macb: use dummy descriptor for RBQP
>      > On 03.12.2020 11:25, Claudiu Beznea wrote:
>      >> In case of multiple queues on RX side the queue scheduler
>      >> will try to use all the available configured queues (with
>      >> descriptors having TX_USED bit cleared). If at least one RBQP
>      >> points to a descriptor with a valid used bit configuration then
>      >> the reception may block as this may point to any memory. To avoid
>      >> this scenario all the queues (except queue zero) were disabled by
>      >> setting DMA descriptors with used bit set on proper RBQP. The driver
>      >> anyway uses only queue 0 for TX/RX.
>      >>
>      >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com
>     <mailto:claudiu.beznea@microchip.com>>
>      >> ---
>      >
>      > Hi Anup, Bin, Padmarao,
>      >
>      > I noticed on the mailing list that you have been actively working and
>      > testing the Macb driver on various platforms, we have this series
>      > outstanding and I want to make sure that it does not break
>     anything on
>      > your side, so it would be appreciated if you could have a look or
>     test
>      > it before it goes into master branch.
>      >
>      > Thanks !
>      > Eugen
>      >
>      >
>      >>   drivers/net/macb.c | 4 +++-
>      >>   drivers/net/macb.h | 2 ++
>      >>   2 files changed, 5 insertions(+), 1 deletion(-)
>      >>
>      >> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
>      >> index b80a259ff757..836eb85ec96a 100644
>      >> --- a/drivers/net/macb.c
>      >> +++ b/drivers/net/macb.c
>      >> @@ -732,8 +732,10 @@ static int gmac_init_multi_queues(struct
>     macb_device *macb)
>      >>        flush_dcache_range(macb->dummy_desc_dma,
>     macb->dummy_desc_dma +
>      >>                        ALIGN(MACB_TX_DUMMY_DMA_DESC_SIZE,
>     PKTALIGN));
>      >>
>      >> -     for (i = 1; i < num_queues; i++)
>      >> +     for (i = 1; i < num_queues; i++) {
>      >>                gem_writel_queue_TBQP(macb, macb->dummy_desc_dma,
>     i - 1);
>      >> +             gem_writel_queue_RBQP(macb, macb->dummy_desc_dma,
>     i - 1);
>      >> +     }
>      >>
>      >>        return 0;
>      >>   }
>      >> diff --git a/drivers/net/macb.h b/drivers/net/macb.h
>      >> index 9b16383eba46..28c7fe306883 100644
>      >> --- a/drivers/net/macb.h
>      >> +++ b/drivers/net/macb.h
>      >> @@ -768,5 +768,7 @@
>      >>   #define GEM_RX_CSUM_CHECKED_MASK            2
>      >>   #define gem_writel_queue_TBQP(port, value, queue_num)       \
>      >>        writel((value), (port)->regs + GEM_TBQP(queue_num))
>      >> +#define gem_writel_queue_RBQP(port, value, queue_num)        \
>      >> +     writel((value), (port)->regs + GEM_RBQP(queue_num))
>      >>
>      >>   #endif /* __DRIVERS_MACB_H__ */
>      >>
>      >
>
Padmarao Begari Jan. 15, 2021, 12:26 p.m. UTC | #8
Hi Eugen,

On Fri, Jan 15, 2021 at 1:34 PM <Eugen.Hristev@microchip.com> wrote:

> On 15.01.2021 06:02, Padmarao Begari wrote:
> > Hi Eugen,
> >
> > On Thu, Jan 14, 2021 at 4:50 PM <Eugen.Hristev@microchip.com
> > <mailto:Eugen.Hristev@microchip.com>> wrote:
> >
> >     On 17.12.2020 07:22, Padmarao Begari - I30397 wrote:
> >      > Hi Eugen,
> >      >
> >      > This series of patches break my side of work(patches) so you need
> to
> >      > create patches after my patches are going into master branch
> >     because my
> >      > patches are already reviewed and tested.
> >
> >     Hi,
> >
> >     Could you please detail the breakage ?
> >
> >
> > The breakage is the fdt relocation disabled in the board environment
> > variables so I have removed it and enabled fdt relocation in PATCH v9.
>
> Maybe you misunderstand my question. I was asking about the sama7g5 macb
> series, which you claimed that breaks your current patch set.
> This is a link to the series :
> https://patchwork.ozlabs.org/project/uboot/list/?series=218367
>
> Since you claimed that this series breaks your series, I am asking what
> exactly is the breakage. How does the fdt relocation in your board
> environment has anything to do with macb and these patches which are not
> applied ?
>
>
My mistake, misunderstood your question,
Yes the fdt relocation has nothing to do with the macb.
We both are adding a member into struct mac_config, a dummy descriptor
for RBQP and my changes are both 32-bit and 64-bit DMA.

Regards
Padmarao


> Thanks,
> Eugen
>
> >
> > Regards
> > Padmarao
> >
> >     I saw a pull request with your patches that was NAK-ed, if your two
> >     macb
> >     patches are tested and reviewed I could apply them to the atmel tree
> as
> >     well and send them, if your PR is delayed. But we are interested to
> >     have
> >     our sama7g5 series pushed as well, so we need to know if it's ok on
> >     your
> >     side, and what is wrong with the sama7g5 series.
> >
> >     Thanks!
> >     Eugen
> >      >
> >      > Regards
> >      > Padmarao
> >      >
> >
>  ------------------------------------------------------------------------
> >      > *From:* Eugen Hristev - M18282 <Eugen.Hristev@microchip.com
> >     <mailto:Eugen.Hristev@microchip.com>>
> >      > *Sent:* Wednesday, December 16, 2020 12:24 PM
> >      > *To:* anup.patel@wdc.com <mailto:anup.patel@wdc.com>
> >     <anup.patel@wdc.com <mailto:anup.patel@wdc.com>>;
> >     bin.meng@windriver.com <mailto:bin.meng@windriver.com>
> >      > <bin.meng@windriver.com <mailto:bin.meng@windriver.com>>;
> >     Padmarao Begari - I30397
> >      > <Padmarao.Begari@microchip.com
> >     <mailto:Padmarao.Begari@microchip.com>>
> >      > *Cc:* Claudiu Beznea - M18063 <Claudiu.Beznea@microchip.com
> >     <mailto:Claudiu.Beznea@microchip.com>>;
> >      > joe.hershberger@ni.com <mailto:joe.hershberger@ni.com>
> >     <joe.hershberger@ni.com <mailto:joe.hershberger@ni.com>>;
> >     u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>
> >      > <u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>>
> >      > *Subject:* Re: [PATCH 1/6] net: macb: use dummy descriptor for
> RBQP
> >      > On 03.12.2020 11:25, Claudiu Beznea wrote:
> >      >> In case of multiple queues on RX side the queue scheduler
> >      >> will try to use all the available configured queues (with
> >      >> descriptors having TX_USED bit cleared). If at least one RBQP
> >      >> points to a descriptor with a valid used bit configuration then
> >      >> the reception may block as this may point to any memory. To avoid
> >      >> this scenario all the queues (except queue zero) were disabled by
> >      >> setting DMA descriptors with used bit set on proper RBQP. The
> driver
> >      >> anyway uses only queue 0 for TX/RX.
> >      >>
> >      >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com
> >     <mailto:claudiu.beznea@microchip.com>>
> >      >> ---
> >      >
> >      > Hi Anup, Bin, Padmarao,
> >      >
> >      > I noticed on the mailing list that you have been actively working
> and
> >      > testing the Macb driver on various platforms, we have this series
> >      > outstanding and I want to make sure that it does not break
> >     anything on
> >      > your side, so it would be appreciated if you could have a look or
> >     test
> >      > it before it goes into master branch.
> >      >
> >      > Thanks !
> >      > Eugen
> >      >
> >      >
> >      >>   drivers/net/macb.c | 4 +++-
> >      >>   drivers/net/macb.h | 2 ++
> >      >>   2 files changed, 5 insertions(+), 1 deletion(-)
> >      >>
> >      >> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> >      >> index b80a259ff757..836eb85ec96a 100644
> >      >> --- a/drivers/net/macb.c
> >      >> +++ b/drivers/net/macb.c
> >      >> @@ -732,8 +732,10 @@ static int gmac_init_multi_queues(struct
> >     macb_device *macb)
> >      >>        flush_dcache_range(macb->dummy_desc_dma,
> >     macb->dummy_desc_dma +
> >      >>                        ALIGN(MACB_TX_DUMMY_DMA_DESC_SIZE,
> >     PKTALIGN));
> >      >>
> >      >> -     for (i = 1; i < num_queues; i++)
> >      >> +     for (i = 1; i < num_queues; i++) {
> >      >>                gem_writel_queue_TBQP(macb, macb->dummy_desc_dma,
> >     i - 1);
> >      >> +             gem_writel_queue_RBQP(macb, macb->dummy_desc_dma,
> >     i - 1);
> >      >> +     }
> >      >>
> >      >>        return 0;
> >      >>   }
> >      >> diff --git a/drivers/net/macb.h b/drivers/net/macb.h
> >      >> index 9b16383eba46..28c7fe306883 100644
> >      >> --- a/drivers/net/macb.h
> >      >> +++ b/drivers/net/macb.h
> >      >> @@ -768,5 +768,7 @@
> >      >>   #define GEM_RX_CSUM_CHECKED_MASK            2
> >      >>   #define gem_writel_queue_TBQP(port, value, queue_num)       \
> >      >>        writel((value), (port)->regs + GEM_TBQP(queue_num))
> >      >> +#define gem_writel_queue_RBQP(port, value, queue_num)        \
> >      >> +     writel((value), (port)->regs + GEM_RBQP(queue_num))
> >      >>
> >      >>   #endif /* __DRIVERS_MACB_H__ */
> >      >>
> >      >
> >
>
>
Eugen Hristev Jan. 15, 2021, 12:42 p.m. UTC | #9
On 15.01.2021 14:26, Padmarao Begari wrote:
> Hi Eugen,
> 
> On Fri, Jan 15, 2021 at 1:34 PM <Eugen.Hristev@microchip.com 
> <mailto:Eugen.Hristev@microchip.com>> wrote:
> 
>     On 15.01.2021 06:02, Padmarao Begari wrote:
>      > Hi Eugen,
>      >
>      > On Thu, Jan 14, 2021 at 4:50 PM <Eugen.Hristev@microchip.com
>     <mailto:Eugen.Hristev@microchip.com>
>      > <mailto:Eugen.Hristev@microchip.com
>     <mailto:Eugen.Hristev@microchip.com>>> wrote:
>      >
>      >     On 17.12.2020 07:22, Padmarao Begari - I30397 wrote:
>      >      > Hi Eugen,
>      >      >
>      >      > This series of patches break my side of work(patches) so
>     you need to
>      >      > create patches after my patches are going into master branch
>      >     because my
>      >      > patches are already reviewed and tested.
>      >
>      >     Hi,
>      >
>      >     Could you please detail the breakage ?
>      >
>      >
>      > The breakage is the fdt relocation disabled in the board environment
>      > variables so I have removed it and enabled fdt relocation in
>     PATCH v9.
> 
>     Maybe you misunderstand my question. I was asking about the sama7g5
>     macb
>     series, which you claimed that breaks your current patch set.
>     This is a link to the series :
>     https://patchwork.ozlabs.org/project/uboot/list/?series=218367
> 
>     Since you claimed that this series breaks your series, I am asking what
>     exactly is the breakage. How does the fdt relocation in your board
>     environment has anything to do with macb and these patches which are
>     not
>     applied ?
> 
> 
> My mistake, misunderstood your question,
> Yes the fdt relocation has nothing to do with the macb.
> We both are adding a member into struct mac_config, a dummy descriptor 
> for RBQP and my changes are both 32-bit and 64-bit DMA.

Okay, so, could you please tell me why you told us that our sama7g5 
series breaks your solarfire SoC series ?
It would be good for both of us if you could test our sama7g5 series on 
top of your patches, on your platform, such that we know that we do not 
affect anything on your board.

Thanks again,
Eugen

> 
> Regards
> Padmarao
> 
>     Thanks,
>     Eugen
> 
>      >
>      > Regards
>      > Padmarao
>      >
>      >     I saw a pull request with your patches that was NAK-ed, if
>     your two
>      >     macb
>      >     patches are tested and reviewed I could apply them to the
>     atmel tree as
>      >     well and send them, if your PR is delayed. But we are
>     interested to
>      >     have
>      >     our sama7g5 series pushed as well, so we need to know if it's
>     ok on
>      >     your
>      >     side, and what is wrong with the sama7g5 series.
>      >
>      >     Thanks!
>      >     Eugen
>      >      >
>      >      > Regards
>      >      > Padmarao
>      >      >
>      >   
>       ------------------------------------------------------------------------
>      >      > *From:* Eugen Hristev - M18282
>     <Eugen.Hristev@microchip.com <mailto:Eugen.Hristev@microchip.com>
>      >     <mailto:Eugen.Hristev@microchip.com
>     <mailto:Eugen.Hristev@microchip.com>>>
>      >      > *Sent:* Wednesday, December 16, 2020 12:24 PM
>      >      > *To:* anup.patel@wdc.com <mailto:anup.patel@wdc.com>
>     <mailto:anup.patel@wdc.com <mailto:anup.patel@wdc.com>>
>      >     <anup.patel@wdc.com <mailto:anup.patel@wdc.com>
>     <mailto:anup.patel@wdc.com <mailto:anup.patel@wdc.com>>>;
>      > bin.meng@windriver.com <mailto:bin.meng@windriver.com>
>     <mailto:bin.meng@windriver.com <mailto:bin.meng@windriver.com>>
>      >      > <bin.meng@windriver.com <mailto:bin.meng@windriver.com>
>     <mailto:bin.meng@windriver.com <mailto:bin.meng@windriver.com>>>;
>      >     Padmarao Begari - I30397
>      >      > <Padmarao.Begari@microchip.com
>     <mailto:Padmarao.Begari@microchip.com>
>      >     <mailto:Padmarao.Begari@microchip.com
>     <mailto:Padmarao.Begari@microchip.com>>>
>      >      > *Cc:* Claudiu Beznea - M18063
>     <Claudiu.Beznea@microchip.com <mailto:Claudiu.Beznea@microchip.com>
>      >     <mailto:Claudiu.Beznea@microchip.com
>     <mailto:Claudiu.Beznea@microchip.com>>>;
>      >      > joe.hershberger@ni.com <mailto:joe.hershberger@ni.com>
>     <mailto:joe.hershberger@ni.com <mailto:joe.hershberger@ni.com>>
>      >     <joe.hershberger@ni.com <mailto:joe.hershberger@ni.com>
>     <mailto:joe.hershberger@ni.com <mailto:joe.hershberger@ni.com>>>;
>      > u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>
>     <mailto:u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>>
>      >      > <u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>
>     <mailto:u-boot@lists.denx.de <mailto:u-boot@lists.denx.de>>>
>      >      > *Subject:* Re: [PATCH 1/6] net: macb: use dummy descriptor
>     for RBQP
>      >      > On 03.12.2020 11:25, Claudiu Beznea wrote:
>      >      >> In case of multiple queues on RX side the queue scheduler
>      >      >> will try to use all the available configured queues (with
>      >      >> descriptors having TX_USED bit cleared). If at least one RBQP
>      >      >> points to a descriptor with a valid used bit
>     configuration then
>      >      >> the reception may block as this may point to any memory.
>     To avoid
>      >      >> this scenario all the queues (except queue zero) were
>     disabled by
>      >      >> setting DMA descriptors with used bit set on proper RBQP.
>     The driver
>      >      >> anyway uses only queue 0 for TX/RX.
>      >      >>
>      >      >> Signed-off-by: Claudiu Beznea
>     <claudiu.beznea@microchip.com <mailto:claudiu.beznea@microchip.com>
>      >     <mailto:claudiu.beznea@microchip.com
>     <mailto:claudiu.beznea@microchip.com>>>
>      >      >> ---
>      >      >
>      >      > Hi Anup, Bin, Padmarao,
>      >      >
>      >      > I noticed on the mailing list that you have been actively
>     working and
>      >      > testing the Macb driver on various platforms, we have this
>     series
>      >      > outstanding and I want to make sure that it does not break
>      >     anything on
>      >      > your side, so it would be appreciated if you could have a
>     look or
>      >     test
>      >      > it before it goes into master branch.
>      >      >
>      >      > Thanks !
>      >      > Eugen
>      >      >
>      >      >
>      >      >>   drivers/net/macb.c | 4 +++-
>      >      >>   drivers/net/macb.h | 2 ++
>      >      >>   2 files changed, 5 insertions(+), 1 deletion(-)
>      >      >>
>      >      >> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
>      >      >> index b80a259ff757..836eb85ec96a 100644
>      >      >> --- a/drivers/net/macb.c
>      >      >> +++ b/drivers/net/macb.c
>      >      >> @@ -732,8 +732,10 @@ static int gmac_init_multi_queues(struct
>      >     macb_device *macb)
>      >      >>        flush_dcache_range(macb->dummy_desc_dma,
>      >     macb->dummy_desc_dma +
>      >      >>                        ALIGN(MACB_TX_DUMMY_DMA_DESC_SIZE,
>      >     PKTALIGN));
>      >      >>
>      >      >> -     for (i = 1; i < num_queues; i++)
>      >      >> +     for (i = 1; i < num_queues; i++) {
>      >      >>                gem_writel_queue_TBQP(macb,
>     macb->dummy_desc_dma,
>      >     i - 1);
>      >      >> +             gem_writel_queue_RBQP(macb,
>     macb->dummy_desc_dma,
>      >     i - 1);
>      >      >> +     }
>      >      >>
>      >      >>        return 0;
>      >      >>   }
>      >      >> diff --git a/drivers/net/macb.h b/drivers/net/macb.h
>      >      >> index 9b16383eba46..28c7fe306883 100644
>      >      >> --- a/drivers/net/macb.h
>      >      >> +++ b/drivers/net/macb.h
>      >      >> @@ -768,5 +768,7 @@
>      >      >>   #define GEM_RX_CSUM_CHECKED_MASK            2
>      >      >>   #define gem_writel_queue_TBQP(port, value,
>     queue_num)       \
>      >      >>        writel((value), (port)->regs + GEM_TBQP(queue_num))
>      >      >> +#define gem_writel_queue_RBQP(port, value,
>     queue_num)        \
>      >      >> +     writel((value), (port)->regs + GEM_RBQP(queue_num))
>      >      >>
>      >      >>   #endif /* __DRIVERS_MACB_H__ */
>      >      >>
>      >      >
>      >
>
diff mbox series

Patch

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index b80a259ff757..836eb85ec96a 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -732,8 +732,10 @@  static int gmac_init_multi_queues(struct macb_device *macb)
 	flush_dcache_range(macb->dummy_desc_dma, macb->dummy_desc_dma +
 			ALIGN(MACB_TX_DUMMY_DMA_DESC_SIZE, PKTALIGN));
 
-	for (i = 1; i < num_queues; i++)
+	for (i = 1; i < num_queues; i++) {
 		gem_writel_queue_TBQP(macb, macb->dummy_desc_dma, i - 1);
+		gem_writel_queue_RBQP(macb, macb->dummy_desc_dma, i - 1);
+	}
 
 	return 0;
 }
diff --git a/drivers/net/macb.h b/drivers/net/macb.h
index 9b16383eba46..28c7fe306883 100644
--- a/drivers/net/macb.h
+++ b/drivers/net/macb.h
@@ -768,5 +768,7 @@ 
 #define GEM_RX_CSUM_CHECKED_MASK		2
 #define gem_writel_queue_TBQP(port, value, queue_num)	\
 	writel((value), (port)->regs + GEM_TBQP(queue_num))
+#define gem_writel_queue_RBQP(port, value, queue_num)	\
+	writel((value), (port)->regs + GEM_RBQP(queue_num))
 
 #endif /* __DRIVERS_MACB_H__ */