netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Lamparter <chunkeey@gmail.com>
To: Ivan Mikhaylov <ivan@de.ibm.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	Rob Herring <robh@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] net/ibm/emac: wrong bit is used for STA control register write
Date: Mon, 22 Jan 2018 21:29:39 +0100	[thread overview]
Message-ID: <9040166.kDGxHD26M5@debian64> (raw)
In-Reply-To: <20180122190146.22876-1-ivan@de.ibm.com>

On Monday, January 22, 2018 8:01:46 PM CET Ivan Mikhaylov wrote:
> >Something looks wrong here?! The commit message talks about bit 18, 19 and 20.
> >However, 0x0800, 0x1000, 0x2000 and are like bit 11, 12 and 13? Furthermore,
> >what about the EMAC_STACR_STAC_MASK? shouldn't it be 0x1800 now (or delete it
> >since it doesn't look like it's used anywhere?).
> Christian, nope, it's all fine there, it's big endian.
Ok Thanks. I think I found the relevant info on Wikipedia:
<https://en.wikipedia.org/wiki/Bit_numbering#Usage>

"Little-endian CPUs usually employ "LSB 0" bit numbering, however both bit
numbering conventions can be seen in big-endian machines. Some architectures
like SPARC and Motorola 68000 use "LSB 0" bit numbering, while S/390, PowerPC
and PA-RISC use "MSB 0"."

But as far as the kernel (code) is concerned, all architectures (big or
little endian) have the same BIT marco:
<https://elixir.free-electrons.com/linux/v4.15-rc9/source/include/linux/bitops.h>
|#define BIT(nr)			(1UL << (nr))
So if someone tries to #define EMAC_STACR_STAC_WRITE BIT(18) it would be
0x40000 instead. This is where the confusion is coming from. Can you please
at least mention this somewhere that all the bits in the commit message are
in "MSB 0" format? It's confusing enough as it is ;).

> This commit related only
> to write operation, I'll check MASK and see what I can do with that.
Well, the MASK is not used and it now looks odd. So you might as well
delete it. Maybe as well as all the unused EMACX_STACR_STAC_IND_* macros?

Thanks,
Christian

  reply	other threads:[~2018-01-22 20:29 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-22 16:00 [PATCH 2/2] net/ibm/emac: wrong bit is used for STA control register write Ivan Mikhaylov
2018-01-22 16:22 ` Christian Lamparter
2018-01-22 19:01   ` Ivan Mikhaylov
2018-01-22 20:29     ` Christian Lamparter [this message]
2018-01-23 16:44       ` Ivan Mikhaylov

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=9040166.kDGxHD26M5@debian64 \
    --to=chunkeey@gmail.com \
    --cc=davem@davemloft.net \
    --cc=ivan@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=robh@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).