All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
To: linux-sh@vger.kernel.org
Subject: Re: [PATCH v2] sh_eth: use RNC mode for R8A7790/R87791
Date: Mon, 02 Jun 2014 22:26:16 +0000	[thread overview]
Message-ID: <538CFA08.3010302@cogentembedded.com> (raw)

Hello.

On 06/03/2014 01:17 AM, Sergei Shtylyov wrote:

>>>>>      Looks like the early SH2/3 SoCs didn't implement the whole register.
>>>>> Despite that, sh_eth_dev_init() always writes to this register... :-/
>>>>> So far, the RMCR.RNC bit was mostly set for the Gigabit-capable
>>>>> controllers, however that rule wasn't strictly followed. Well, this
>>>>> driver is still a mess, and it's hard to deal with it without the
>>>>> necessary documentation.

>>>> Why don't we therefore:

>>>> 1) Skip the register write if the per-chip value is zero.

>>>     I rather thought about not writing when the register is not
>>>     implemented.
>>> I'll probably look into this when I have time.

>>>> 2) Add the RNC bit to all of the gigabit capable controllers.

>>>     I probably misspoke -- all the Gigabit controllers already have it
>>>     set, it's just that some 100 MBbps ones have it set, but most don't.

>> So these chips that do not implement the register, they only process
>> one RX descriptor at a time until the interrupt handler re-enables
>> DMA receive?

>     I just don't know. Looks like the driver is broken on SH2/3 even more than
> I thought: it always reads the EDRRR register in sh_eth_rx() trying to
> understand if the reception has been stopped but that register doesn't seem to
> exist on SH2/3. Moreover, sh_eth_interrupt() reads EESR in order to determine
> the interrupt status but that register doesn't seem to exist on SH2/3 either!

    OK, I've chased down the commit that broke SH2/3 support 3+ years ago; 
here it is:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idJ55530f38e4eeee3afb06093e81309138fe8360

    All the registers I've mentioned did exist on SH2/3, they just got missed 
in the mapping arrays.

WBR, Sergei


WARNING: multiple messages have this Message-ID (diff)
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
To: David Miller <davem@davemloft.net>
Cc: ben.dooks@codethink.co.uk, linux-kernel@codethink.co.uk,
	netdev@vger.kernel.org, nobuhiro.iwamatsu.yj@renesas.com,
	magnus.damn@opensource.se, horms@verge.net.au,
	yoshihiro.shimoda.uh@renesas.com, cm-hiep@jinso.co.jp,
	linux-sh@vger.kernel.org
Subject: Re: [PATCH v2] sh_eth: use RNC mode for R8A7790/R87791
Date: Tue, 03 Jun 2014 02:26:16 +0400	[thread overview]
Message-ID: <538CFA08.3010302@cogentembedded.com> (raw)
In-Reply-To: <538CE9EB.1010506@cogentembedded.com>

Hello.

On 06/03/2014 01:17 AM, Sergei Shtylyov wrote:

>>>>>      Looks like the early SH2/3 SoCs didn't implement the whole register.
>>>>> Despite that, sh_eth_dev_init() always writes to this register... :-/
>>>>> So far, the RMCR.RNC bit was mostly set for the Gigabit-capable
>>>>> controllers, however that rule wasn't strictly followed. Well, this
>>>>> driver is still a mess, and it's hard to deal with it without the
>>>>> necessary documentation.

>>>> Why don't we therefore:

>>>> 1) Skip the register write if the per-chip value is zero.

>>>     I rather thought about not writing when the register is not
>>>     implemented.
>>> I'll probably look into this when I have time.

>>>> 2) Add the RNC bit to all of the gigabit capable controllers.

>>>     I probably misspoke -- all the Gigabit controllers already have it
>>>     set, it's just that some 100 MBbps ones have it set, but most don't.

>> So these chips that do not implement the register, they only process
>> one RX descriptor at a time until the interrupt handler re-enables
>> DMA receive?

>     I just don't know. Looks like the driver is broken on SH2/3 even more than
> I thought: it always reads the EDRRR register in sh_eth_rx() trying to
> understand if the reception has been stopped but that register doesn't seem to
> exist on SH2/3. Moreover, sh_eth_interrupt() reads EESR in order to determine
> the interrupt status but that register doesn't seem to exist on SH2/3 either!

    OK, I've chased down the commit that broke SH2/3 support 3+ years ago; 
here it is:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4a55530f38e4eeee3afb06093e81309138fe8360

    All the registers I've mentioned did exist on SH2/3, they just got missed 
in the mapping arrays.

WBR, Sergei


             reply	other threads:[~2014-06-02 22:26 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-02 22:26 Sergei Shtylyov [this message]
2014-06-02 22:26 ` [PATCH v2] sh_eth: use RNC mode for R8A7790/R87791 Sergei Shtylyov
2014-06-03 11:19 ` Ben Dooks
2014-06-03 11:19   ` Ben Dooks
2014-06-03 20:02 ` Sergei Shtylyov
2014-06-03 20:02   ` Sergei Shtylyov
  -- strict thread matches above, loose matches on Subject: below --
2014-06-02 17:17 Ben Dooks
2014-06-02 18:53 ` David Miller
2014-06-02 19:06   ` Ben Dooks
2014-06-02 19:33   ` Sergei Shtylyov
2014-06-02 20:49     ` David Miller
2014-06-02 20:55       ` Sergei Shtylyov
2014-06-02 21:05         ` David Miller
2014-06-02 21:17           ` Sergei Shtylyov
2014-06-03  2:45       ` Yoshihiro Shimoda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=538CFA08.3010302@cogentembedded.com \
    --to=sergei.shtylyov@cogentembedded.com \
    --cc=linux-sh@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.