netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Elior, Ariel" <Ariel.Elior@cavium.com>
To: David Miller <davem@davemloft.net>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Kalderon, Michal" <Michal.Kalderon@cavium.com>,
	"Tayar, Tomer" <Tomer.Tayar@cavium.com>
Subject: RE: [PATCH net-next v2 1/6] qed: Add doorbell overflow recovery mechanism
Date: Tue, 23 Oct 2018 05:16:21 +0000	[thread overview]
Message-ID: <SN6PR07MB5024578EC06BDA6E49A4960390F50@SN6PR07MB5024.namprd07.prod.outlook.com> (raw)
In-Reply-To: <20181022.200559.1672641132565115543.davem@davemloft.net>

>
> > +#ifndef writeq
> > +#define writeq writeq
> > +static inline void writeq(u64 val, void __iomem *reg)
> > +{
> > +     writel(val & 0xffffffff, reg);
> > +     writel(val >> 32, reg + 0x4UL);
> > +}
> > +#endif
> 
> Please use the appropriate generic header file to achieve this, do not
> reimplement it.
> 
> See include/linux/io-64-nonatomic*.h and think very carefully about
> which one is appropriate.
> 
> Specifically, if a register read has side effects but only if you read
> the lower or upper half you want to make sure that you use the
> implementation that reads the half the doesn't trigger the side
> effects first.  This way the whole 64-bit value can be sampled before
> status bits clear, or whatever.
> 
> Likewise for which half of a register, when written, commits the
> results.
> 
> If both halfs trigger the side effect or the commit of the write, you
> cannot enable your driver on 32-bit architectures.
> 
> So this is not such a simple fix where you just hack the build to pass,
> you have to really think hard about what the code does, how the hardware
> works, and if this can even work properly on 32-bit platforms.
> 
> Thank you.

Thanks for pointing out the generic implementation. In our drivers we only use
The 64b macros for a single purpose - writing the rdma CQ doorbells. It will not
be used in any other register access. In this case the "lower and then upper" is
the correct behavior for us. I will send a v3 using the generic implementation.

Thanks,
Ariel

  reply	other threads:[~2018-10-23 13:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-22 16:40 [PATCH net-next 0/6 v2] qed*: Doorbell overflow recovery Ariel Elior
2018-10-22 16:40 ` [PATCH net-next v2 1/6] qed: Add doorbell overflow recovery mechanism Ariel Elior
2018-10-23  3:05   ` David Miller
2018-10-23  5:16     ` Elior, Ariel [this message]
2018-10-22 16:40 ` [PATCH net-next v2 2/6] qed: Use the doorbell overflow recovery mechanism in case of doorbell overflow Ariel Elior
2018-10-22 16:40 ` [PATCH net-next v2 3/6] qed: Register slowpath queue doorbell with doorbell overflow recovery mechanism Ariel Elior
2018-10-22 16:40 ` [PATCH net-next v2 4/6] qed: Register light L2 queues " Ariel Elior
2018-10-22 16:40 ` [PATCH net-next v2 5/6] qed: Expose the doorbell overflow recovery mechanism to the protocol drivers Ariel Elior
2018-10-22 16:40 ` [PATCH net-next v2 6/6] qede: Register l2 queues with doorbell overflow recovery mechanism Ariel Elior

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=SN6PR07MB5024578EC06BDA6E49A4960390F50@SN6PR07MB5024.namprd07.prod.outlook.com \
    --to=ariel.elior@cavium.com \
    --cc=Michal.Kalderon@cavium.com \
    --cc=Tomer.Tayar@cavium.com \
    --cc=davem@davemloft.net \
    --cc=netdev@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 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).