All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Marc Kleine-Budde' <mkl@pengutronix.de>,
	'Geert Uytterhoeven' <geert@linux-m68k.org>
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"wg@grandegger.com" <wg@grandegger.com>,
	"linux-can@vger.kernel.org" <linux-can@vger.kernel.org>,
	Linux-sh list <linux-sh@vger.kernel.org>,
	Pavel Kiryukhin <vksavl@gmail.com>
Subject: RE: [PATCH v5] can: add Renesas R-Car CAN driver
Date: Fri, 28 Feb 2014 12:02:52 +0000	[thread overview]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6D0F6CDC10@AcuExch.aculab.com> (raw)
In-Reply-To: <5310781F.1000005@pengutronix.de>

RnJvbTogTWFyYyBLbGVpbmUtQnVkZGUNCj4gT24gMDIvMjgvMjAxNCAxMjo0NyBQTSwgRGF2aWQg
TGFpZ2h0IHdyb3RlOg0KPiA+IEZyb206IEdlZXJ0IFV5dHRlcmhvZXZlbg0KPiA+PiBPbiBGcmks
IEZlYiAyOCwgMjAxNCBhdCAxMjozNyBQTSwgTWFyYyBLbGVpbmUtQnVkZGUgPG1rbEBwZW5ndXRy
b25peC5kZT4gd3JvdGU6DQo+ID4+Pj4+IEEgMzIgYml0IHJlYWQvbW9kaWZ5L3dyaXRlIGlzIGEg
c3RhbmRhcmQgb3BlcmF0aW9uLCBub3RoaW5nIHNwZWNpYWwsIG5vDQo+ID4+Pj4+IG5lZWQgdG8g
d29ycnkgYWJvdXQgYnl0ZSBzd2FwcGluZyBvciBhbnl0aGluZyBsaWtlIHRoaXMuDQo+ID4+Pj4N
Cj4gPj4+PiAgICBPaCwgcmVhbGx5PyA4LSkNCj4gPj4+PiAgICBEb24ndCB5b3Uga25vdyB0aGF0
IHJlYWRbYndscV0oKSBhc3N1bWUgbGl0dGxlLWVuZGlhbiBtZW1vcnkgbGF5b3V0DQo+ID4+Pj4g
YW5kIHRvIHJlYWQgZnJvbSBiaWctZW5kaWFuIDMyLWJpdCByZWdpc3RlciBvbmUgbm9ybWFsbHkg
bmVlZHMgcmVhZGxfYmUoKT8NCj4gPj4+DQo+ID4+PiBJIGFzc3VtZSB5b3UgYXJlIG9uIGxpdHRs
ZSBlbmRpYW4gQVJNIG9ubHkgKGZvciBub3cpLg0KPiA+Pj4NCj4gPj4+IElmIHlvdSB1c2UgYSBz
dGFuZGFyZCAzMiBiaXQgcmVhZCwgdGhlbiBtb2RpZnkgdGhlIGNvcnJlY3QgYml0cyBpbiB0aGF0
DQo+ID4+PiAzMiBiaXQgd29yZCBhbmQgd3JpdGUgaXQgYmFjaywgd2l0aCB0aGUgY29ycmVzcG9u
ZGluZyAzMiBiaXQgd3JpdGUNCj4gPj4+IGV2ZXJ5dGhpbmcgc2hvdWxkIGJlIGZpbmUuIEZvciB0
aGlzIHVzZWNhc2UgeW91IGp1c3QgaGF2ZSB5byBmaWd1cmUgb3V0DQo+ID4+PiB3aGljaCAyNCBv
ZiB0aGUgMzIgYml0IGFyZSB0aGUgb25lIHlvdSBoYXZlIHRvIGNoYW5nZSBhbmQgd2hpY2ggYXJl
IHRoZQ0KPiA+Pj4gOCB0aGF0IG11c3Qgbm90IGJlIG1vZGlmaWVkLg0KPiA+Pj4NCj4gPj4+IExv
b2tpbmcgYXQgdGhlIHJlZ2lzdGVyIGxheW91dDoNCj4gPj4+DQo+ID4+Pj4gKyAgICAgdTggYmNy
WzNdOyAgICAgIC8qIEJpdCBDb25maWd1cmF0aW9uIFJlZ2lzdGVyICovDQo+ID4+Pj4gKyAgICAg
dTggY2xrcjsgICAgICAgIC8qIENsb2NrIFNlbGVjdCBSZWdpc3RlciAqLw0KPiA+Pj4NCj4gPj4+
IEkgdGhpbmsgY2xrciB3b3VsZCBiZSB0aGUgbG93ZXN0IDggYml0IGFuZCBiY3JbXSBhcmUgdGhl
IHVwcGVyIDI0Lg0KPiA+Pg0KPiA+PiBUaGF0IHdvdWxkIGJlIHRoZSBvdXRjb21lIG9uIGJpZyBl
bmRpYW4gOy0pDQo+ID4NCj4gPiBMb29rcyB0byBtZSBhcyB0aG91Z2ggaXQgc2hvdWxkIGJlIGRl
ZmluZWQgYXMgYSAzMmJpdCBmaWVsZCBhbmQgdGhlbg0KPiA+IHRoZSBhcHByb3ByaWF0ZSBiaXQg
ZGVmaW5pdGlvbnMgYW5kIG1hc2tzIGFwcGxpZWQuDQo+IA0KPiBBY2ssIDMyYml0IHllcywgYnV0
IG5vIGZpZWxkIChhcyBpbiBodHRwOi8vZW4ud2lraXBlZGlhLm9yZy93aWtpL0JpdF9maWVsZCku
DQoNClllcywgSSBtZWFudCAnZmllbGQnIGFzIGluICdzdHJ1Y3R1cmUgbWVtYmVyJywgbm90IGJp
dC1maWVsZC4NCkkgbGVhcm50IGEgbG9uZyB0aW1lIGFnbyB0aGF0IGJpdC1maWVsZHMgYXJlIHJh
cmVseSwgaWYgZXZlciwgdGhlIGNvbnN0cnVjdA0KeW91IGFyZSBsb29raW5nIGZvci4NCjEpIEEg
cGFja2VkIGxvb2t1cCB0YWJsZSB3aGVyZSB0aGUgY29kZSBmb3Igb25lIGFjY2VzcyB3YXMgbGFy
Z2VyIHRoYW4gdGhlIHRhYmxlLg0KMikgZm9vLT50d29fYml0X2ZpZWxkID0gMzsgY29tcGlsZWQg
aW50byBjb2RlIHRoYXQgZmlyc3QgemVyb2VkIHRoZSB0d28gYml0cw0KICAgKGNhdXNlZCBhbiBJ
U1IgdG8gY29ycnVwdCBhIGxpbmtlZCBsaXN0KS4NCk5ldmVyIG1pbmQgdGhlIGlzc3VlcyBhYm91
dCB0aGUgYWN0dWFsIG9yZGVyIG9mIHRoZSBiaXRzLg0KDQoJRGF2aWQNCg0K

WARNING: multiple messages have this Message-ID (diff)
From: David Laight <David.Laight@ACULAB.COM>
To: 'Marc Kleine-Budde' <mkl@pengutronix.de>,
	'Geert Uytterhoeven' <geert@linux-m68k.org>
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"wg@grandegger.com" <wg@grandegger.com>,
	"linux-can@vger.kernel.org" <linux-can@vger.kernel.org>,
	Linux-sh list <linux-sh@vger.kernel.org>,
	Pavel Kiryukhin <vksavl@gmail.com>
Subject: RE: [PATCH v5] can: add Renesas R-Car CAN driver
Date: Fri, 28 Feb 2014 12:02:52 +0000	[thread overview]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6D0F6CDC10@AcuExch.aculab.com> (raw)
In-Reply-To: <5310781F.1000005@pengutronix.de>

From: Marc Kleine-Budde
> On 02/28/2014 12:47 PM, David Laight wrote:
> > From: Geert Uytterhoeven
> >> On Fri, Feb 28, 2014 at 12:37 PM, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> >>>>> A 32 bit read/modify/write is a standard operation, nothing special, no
> >>>>> need to worry about byte swapping or anything like this.
> >>>>
> >>>>    Oh, really? 8-)
> >>>>    Don't you know that read[bwlq]() assume little-endian memory layout
> >>>> and to read from big-endian 32-bit register one normally needs readl_be()?
> >>>
> >>> I assume you are on little endian ARM only (for now).
> >>>
> >>> If you use a standard 32 bit read, then modify the correct bits in that
> >>> 32 bit word and write it back, with the corresponding 32 bit write
> >>> everything should be fine. For this usecase you just have yo figure out
> >>> which 24 of the 32 bit are the one you have to change and which are the
> >>> 8 that must not be modified.
> >>>
> >>> Looking at the register layout:
> >>>
> >>>> +     u8 bcr[3];      /* Bit Configuration Register */
> >>>> +     u8 clkr;        /* Clock Select Register */
> >>>
> >>> I think clkr would be the lowest 8 bit and bcr[] are the upper 24.
> >>
> >> That would be the outcome on big endian ;-)
> >
> > Looks to me as though it should be defined as a 32bit field and then
> > the appropriate bit definitions and masks applied.
> 
> Ack, 32bit yes, but no field (as in http://en.wikipedia.org/wiki/Bit_field).

Yes, I meant 'field' as in 'structure member', not bit-field.
I learnt a long time ago that bit-fields are rarely, if ever, the construct
you are looking for.
1) A packed lookup table where the code for one access was larger than the table.
2) foo->two_bit_field = 3; compiled into code that first zeroed the two bits
   (caused an ISR to corrupt a linked list).
Never mind the issues about the actual order of the bits.

	David


  reply	other threads:[~2014-02-28 12:02 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-26 20:37 [PATCH v5] can: add Renesas R-Car CAN driver Sergei Shtylyov
2013-12-26 21:37 ` Sergei Shtylyov
2014-01-13 13:46 ` Sergei Shtylyov
2014-01-13 13:46   ` Sergei Shtylyov
2014-01-20  9:18 ` Marc Kleine-Budde
2014-01-20  9:18   ` Marc Kleine-Budde
2014-01-25  0:34   ` Sergei Shtylyov
2014-01-25  1:34     ` Sergei Shtylyov
2014-02-13 12:12     ` Marc Kleine-Budde
2014-02-13 12:12       ` Marc Kleine-Budde
2014-02-20 22:48       ` Sergei Shtylyov
2014-02-20 23:48         ` Sergei Shtylyov
2014-02-28  9:08         ` Marc Kleine-Budde
2014-02-28  9:08           ` Marc Kleine-Budde
2014-02-28 11:16           ` Sergei Shtylyov
2014-02-28 11:16             ` Sergei Shtylyov
2014-02-28 11:37             ` Marc Kleine-Budde
2014-02-28 11:37               ` Marc Kleine-Budde
2014-02-28 11:41               ` Geert Uytterhoeven
2014-02-28 11:41                 ` Geert Uytterhoeven
2014-02-28 11:47                 ` David Laight
2014-02-28 11:47                   ` David Laight
2014-02-28 11:50                   ` Marc Kleine-Budde
2014-02-28 11:50                     ` Marc Kleine-Budde
2014-02-28 12:02                     ` David Laight [this message]
2014-02-28 12:02                       ` David Laight
2014-02-28 11:49                 ` Marc Kleine-Budde
2014-02-28 11:49                   ` Marc Kleine-Budde
2014-02-28 12:05                   ` Sergei Shtylyov
2014-02-28 12:05                     ` Sergei Shtylyov
2014-02-28 12:17                     ` David Laight
2014-02-28 12:17                       ` David Laight
2014-02-28 12:34                       ` Sergei Shtylyov
2014-02-28 12:34                         ` Sergei Shtylyov
2014-01-20 11:43 ` Geert Uytterhoeven
2014-01-20 11:43   ` Geert Uytterhoeven
2014-01-20 11:47   ` Marc Kleine-Budde
2014-01-20 11:47     ` Marc Kleine-Budde
2014-01-20 11:52     ` Geert Uytterhoeven
2014-01-20 11:52       ` Geert Uytterhoeven
2014-01-20 11:58       ` Marc Kleine-Budde
2014-01-20 11:58         ` Marc Kleine-Budde
2014-01-20 12:02         ` Ben Dooks
2014-01-20 12:05           ` Geert Uytterhoeven
2014-01-20 12:05             ` Geert Uytterhoeven
2014-01-20 12:08             ` Marc Kleine-Budde
2014-01-20 12:08               ` Marc Kleine-Budde
2014-01-20 12:05           ` Marc Kleine-Budde
2014-01-20 12:05             ` Marc Kleine-Budde
2014-01-20 12:13           ` David Laight
2014-01-20 12:13             ` David Laight
2014-01-20 12:35             ` Marc Kleine-Budde
2014-01-20 12:35               ` Marc Kleine-Budde
2014-01-20 19:16         ` David Miller
2014-01-20 19:16           ` David Miller
2014-01-20 21:12           ` Sergei Shtylyov
2014-01-20 22:12             ` Sergei Shtylyov
2014-01-20 21:17             ` Marc Kleine-Budde
2014-01-20 21:17               ` Marc Kleine-Budde
2014-01-22 11:52               ` Ben Dooks
2014-01-22 11:54                 ` Geert Uytterhoeven
2014-01-22 11:54                   ` Geert Uytterhoeven
2014-01-22 11:58                 ` David Laight
2014-01-22 11:58                   ` David Laight
2014-01-20 12:12   ` Sergei Shtylyov
2014-01-20 12:12     ` Sergei Shtylyov

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=063D6719AE5E284EB5DD2968C1650D6D0F6CDC10@AcuExch.aculab.com \
    --to=david.laight@aculab.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=vksavl@gmail.com \
    --cc=wg@grandegger.com \
    /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.