LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Joel Stanley <joel@jms.id.au>
To: Florent Kermarrec <florent@enjoy-digital.fr>
Cc: "Gabriel L. Somlo" <gsomlo@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Stafford Horne <shorne@gmail.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Anton Blanchard <anton@ozlabs.org>, David Shah <dave@ds0.me>,
	Karol Gugala <kgugala@antmicro.com>,
	Mateusz Holenko <mholenko@antmicro.com>,
	devicetree <devicetree@vger.kernel.org>,
	Networking <netdev@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] net: Add driver for LiteX's LiteETH network interface
Date: Fri, 20 Aug 2021 07:46:27 +0000	[thread overview]
Message-ID: <CACPK8Xds9RysihvNSJfxcL4n1-eum3v1gauh406jEfdu=S7pcw@mail.gmail.com> (raw)
In-Reply-To: <CA+96J6JxAkNd_QkVxfSdAJwWVLJTtSaDsCmVKw3KBfYySJytKA@mail.gmail.com>

Hi Florent and Gabriel,

On Fri, 6 Aug 2021 at 12:27, Florent Kermarrec <florent@enjoy-digital.fr> wrote:
>
> Hi Gabriel, Joel,
>
> for 1), the polling mode has been useful recently with Linux-On-LiteX-Vexriscv to bring up the SMP version before enabling the IRQs and can probably be useful in the future for similar purposes. So if it's not too complicated to maintain, this could be useful to keep it.

I would prefer to get a basic working driver for the straight forward
Linux-on-FPGA use case merged first. We can then work out what should
be done next. I would like to see hardware improvements to make it go
faster, and we can look at other features like polling if you still
need them.

>
> Le ven. 6 août 2021 à 13:48, Gabriel L. Somlo <gsomlo@gmail.com> a écrit :
>>
>> Hi Joel,
>>
>> Thanks for pushing this upstream (and for writing it to begin with)!
>>
>> Would you mind diff-ing your version of litex_liteeth.c against what
>> is currently in
>> https://github.com/litex-hub/linux/blob/litex-rebase/drivers/net/ethernet/litex/litex_liteeth.c ?
>>
>> Two main differences we should discuss:
>>
>>         1. there's a polling mode (added by Antony Pavlov), and if we
>>            decide *not* to keep it around, I want to ensure we do that
>>            deliberately, with an explanation as to why;

I assumed this was only for broken systems, or systems that were still
being brought up. Haven't the risc-v socs improved to the point where
they have working interrupts now?

>>
>>         2. LiteX CSRs are accessed using `litex_[read|write][8|16|32]()`
>>            as opposed to simply `[read|write][b|w|l]()`. The former set
>>            are defined in `include/linux/litex.h` and are needed to
>>            ensure correct accesses regardless of endianness, since by
>>            default LiteX registers' endianness mirrors that of the
>>            configured CPU.

I don't like that they get the parameters in the wrong order compared
to the rest of the kernel, and that they force 32-bit access no matter
the width of the access being performed. They also mean we can't use
_releaxed variants of accessors to eg. read a set of registers without
having a barrier in between.

That said, I have used the csr accessors so we can support the mor1k,
which afaict is the only litex CPU that defaults to big endian.

If you could help review the patch so we can get it merged this cycle,
then we can work on future enhancements.

Cheers,

Joel

  parent reply	other threads:[~2021-08-20  7:46 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-06  5:49 [PATCH 0/2] net: Add LiteETH network driver Joel Stanley
2021-08-06  5:49 ` [PATCH 1/2] dt-bindings: net: Add bindings for LiteETH Joel Stanley
2021-08-06 17:07   ` Rob Herring
2021-08-07 19:05   ` Andrew Lunn
2021-08-09  7:59     ` Joel Stanley
2021-08-09 13:27       ` Andrew Lunn
2021-08-06  5:49 ` [PATCH 2/2] net: Add driver for LiteX's LiteETH network interface Joel Stanley
2021-08-06 11:48   ` Gabriel L. Somlo
     [not found]     ` <CA+96J6JxAkNd_QkVxfSdAJwWVLJTtSaDsCmVKw3KBfYySJytKA@mail.gmail.com>
2021-08-20  7:46       ` Joel Stanley [this message]
2021-08-06 23:10   ` Jakub Kicinski
2021-08-09 12:03     ` Joel Stanley
2021-08-09 16:16       ` Jakub Kicinski
2021-08-07 19:19   ` Andrew Lunn
2021-08-09  3:20     ` Joel Stanley
2021-08-09 13:09       ` Andrew Lunn

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='CACPK8Xds9RysihvNSJfxcL4n1-eum3v1gauh406jEfdu=S7pcw@mail.gmail.com' \
    --to=joel@jms.id.au \
    --cc=anton@ozlabs.org \
    --cc=benh@kernel.crashing.org \
    --cc=dave@ds0.me \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=florent@enjoy-digital.fr \
    --cc=gsomlo@gmail.com \
    --cc=kgugala@antmicro.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mholenko@antmicro.com \
    --cc=netdev@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=shorne@gmail.com \
    --subject='Re: [PATCH 2/2] net: Add driver for LiteX'\''s LiteETH network interface' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).